Skip to content

fix: accept query token on dashboard settle POST (rebased, supersedes #27) - #45

Merged
linhdmn merged 1 commit into
mainfrom
land/pr27-settle-query-token
Oct 7, 2026
Merged

linhdmn merged 1 commit into
mainfrom
land/pr27-settle-query-token

Conversation

@linhdmn

@linhdmn linhdmn commented Oct 7, 2026

Copy link
Copy Markdown
Member

Summary

Supersedes #27, which has been CONFLICTING against main since 2026-09-30.

#27's branch (dsh/dashboard-settle-query-token) is 1 commit ahead / 32 behind main, and the conflict is a real one: main grew grantOwnAsk (run-scoped approval, #43) directly above the approve definition the fix rewrites. Merging the stale branch as-is would need that resolved by hand anyway, so this branch is the same commit rebased onto 3534097 (v0.4.2) with the conflict resolved — nothing else changed.

The change (unchanged from #27)

  • POST /api/approvals/:id now receives the real request URL and passes allowQuery: true, so ?token= works the same way the GET dashboard routes do.
  • The x-dashboard-token header still wins when present.
  • sameOrigin still rejects cross-origin browser POSTs — the state-changing route keeps both gates.
  • Closes KNOWN-ISSUES §5, which moves from "Open — product warts" to "Fixed".
  • CHANGELOG.md gets the entry under [Unreleased] → Fixed, appended after the existing entries rather than opening a second ### Fixed block.

The conflict resolution

Only src/dashboard.ts and CHANGELOG.md conflicted.

  • src/dashboard.ts: kept main's grantOwnAsk verbatim, and replaced only the old one-line approve signature plus its authorized(req, new URL('/', 'http://x'), false) body line with the fix's four-parameter signature and authorized(req, url, true).
  • CHANGELOG.md: kept main's whole ### Added section and appended the one new Fixed bullet.

Test plan

Run in this worktree on 3534097 + c82d674:

  • node --experimental-strip-types --test test/*.test.ts — 819 pass, 0 fail, 1 skipped (820 tests)
  • node --experimental-strip-types --test test/dashboard.test.ts — 40/40, including POST settle accepts ?token= without the header (KNOWN-ISSUES §5)
  • npx tsc --noEmit — clean
  • bash .githooks/pre-commit --all — security scan ok (all)
  • Re-verified after the rebase: the new test passes on the merged tree, not only on the stale branch

The live-model evidence from #27 (docs/evidence/settle-query-token-live/) is carried over unchanged — it was measured against this same code, and the rebase did not touch approve's behaviour.

Once this lands, #27 and its branch can be closed and deleted.

POST /api/approvals/:id discarded ?token= by authorizing against a
placeholder URL with allowQuery false, so scripted clients got a 401 that
looked like a wrong secret. Pass the real request URL and allow query tokens
the same way GET routes do; header auth and sameOrigin still apply.
@linhdmn
linhdmn merged commit 2cc8737 into main Oct 7, 2026
3 checks passed
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.

1 participant