Skip to content

fix: require application/json on dashboard POST endpoints - #6

Closed
SashaMIT wants to merge 1 commit into
AMAP-ML:mainfrom
SashaMIT:fix/dashboard-csrf-content-type
Closed

fix: require application/json on dashboard POST endpoints#6
SashaMIT wants to merge 1 commit into
AMAP-ML:mainfrom
SashaMIT:fix/dashboard-csrf-content-type

Conversation

@SashaMIT

@SashaMIT SashaMIT commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The dashboard control plane (/api/approvals/<id>/resolve, /api/inject, /api/select-run) drives a running agent: it resolves pending approvals and injects operator instructions.
  • _read_json_body parses the payload regardless of Content-Type, and browsers may send cross-origin "simple" POSTs (text/plain/form-encoded) without a CORS preflight. Any page the operator visits while the dashboard is up could therefore inject instructions or resolve approvals on their behalf.
  • Require an application/json Content-Type on all POSTs. Cross-origin attempts then become preflighted requests, which this server never answers. The bundled UI already sends application/json on every POST, so nothing changes for legitimate use.

Test plan

  • tests/test_dashboard_server.py: POST with text/plain / no Content-Type → 415; POST with application/json (with/without charset) → accepted.

Made with Cursor

Made with Cursor

@lerogo

lerogo commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the contribution! Could you please simplify this PR before we merge it?

  • Remove uv.lock and the added tests; they are not needed for this small change.
  • This is a local-only dashboard, so please keep the implementation minimal.
  • Do not validate Content-Type before routing, as unknown paths would return 415 instead of 404.
  • Avoid startswith("application/json"), which also accepts invalid values such as application/json-invalid.

Please keep the change focused and preserve the existing routing behavior. Thanks again!

The dashboard's approval-resolve and instruction-inject routes drive a
running agent, and previously parsed request bodies regardless of
Content-Type. Browsers may send cross-origin 'simple' POSTs (text/plain
or form-encoded) without a CORS preflight, so any page the operator
visited while the dashboard was up could resolve pending approvals or
inject instructions on their behalf.

Requiring an exact application/json content type on those two routes
turns every cross-origin attempt into a preflighted request, which this
server never answers. The bundled UI already sends application/json.
The check lives after routing so unknown paths keep their 404, and the
comparison is exact so lookalike types are not accepted.
@SashaMIT
SashaMIT force-pushed the fix/dashboard-csrf-content-type branch from e9b4809 to a63847c Compare August 7, 2026 03:17
@SashaMIT

SashaMIT commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, all four points addressed and pushed:

  • Removed the tests and uv.lock; the PR now touches only server.py.
  • The check runs after routing, so unknown paths keep their 404.
  • Switched to an exact Content-Type: application/json comparison, so values like application/json-invalid are rejected.
  • Scope narrowed to the two routes that drive a running agent (approval resolve + instruction inject); /api/select-run is browse-only state and left untouched.

Verified live: text/plain gets 415, unknown paths still 404, and the bundled UI already sends bare application/json on both guarded routes, so it is unaffected.

@SashaMIT

SashaMIT commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #11, which adds the loopback Host gate, covers select-run, and allows charset on application/json. Closing this incomplete CSRF-only PR in favor of that one.

@SashaMIT SashaMIT closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants