Skip to content

Keep bearer URL tokens out of logs and run views - #2113

Open
ppXD wants to merge 1 commit into
fix/bind-model-endpoints-to-their-keysfrom
fix/keep-bearer-url-tokens-out-of-logs-and-views
Open

ppXD wants to merge 1 commit into
fix/bind-model-endpoints-to-their-keysfrom
fix/keep-bearer-url-tokens-out-of-logs-and-views

Conversation

@ppXD

@ppXD ppXD commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Problem. Invitation, password-reset and workflow-callback tokens travel in URL paths. Every request wrote its raw path to the console and Seq: Serilog's completion line, the hosting scope's RequestPath on every event, and GlobalExceptionFilter. The SPA's nginx logged the request line and the Referer, and a page opened at /invite/<token> sends that URL as the Referer of every asset. Anyone who could read logs could join a team, reset a password or resume a run. Separately, run detail and pending-wait projected a Callback wait's token to every team member, so a Viewer could resume a run anonymously with a body of their choosing and bypass runs.control.

  • Redaction where a bearer route answers. A route that binds a token parameter is logged by its template (/api/invitations/{token}/accept). BearerRouteLogging (backend/src/CodeSpace.Api/Logging/BearerRouteLogging.cs) does this and runs first in the pipeline (Startup.cs).

  • Redaction where no bearer route answers. Some requests never reach a bearer route:

    • a GET or HEAD on a callback URL;
    • a wrong method;
    • the SPA's /invite/… or /reset-password/… link opened on the API origin;
    • anything logged before routing, such as UseHttpsRedirection's port warning.

    For these, a path with a bearer route's shape is logged with that segment as {token}. The shapes come from the host's endpoint table plus the SPA routes that PublicBaseUrlSetting builds links on.

  • nginx. frontend/nginx.conf logs {token} in place of the segment in both the request line and the Referer. URL shapes are unchanged, and every issued link keeps working.

  • Callback token out of run reads. For a Callback wait, GET /api/workflows/runs/{id} and GET /api/workflows/runs/{id}/pending-wait now send the fixed placeholder withheld-reload-the-page-to-reveal (WorkflowWaitKinds.WithheldBearerToken) where the token used to be. Other wait kinds are unchanged.

    • It is not null because the pending-wait decoder in every shipped SPA refuses anything but a 1..128-character string, so an older SPA would have shown "Pending action unavailable".
    • On an older SPA the run page builds a URL from the placeholder, and that URL resumes nothing. It is 34 characters long, the callback route admits only {token:length(32)}, and no version mints that shape.
  • Reveal endpoint. POST /api/workflows/runs/{runId}/waits/{waitId}/callback-url (RevealWorkflowCallbackUrlCommand) returns { waitId, path }.

    • It requires runs.control. Anyone else gets 403.
    • It is team-scoped. A foreign run, a wait that is not pending, or a wait that is not a Callback all return 404.
    • It logs who revealed the URL, at Warning.
    • It reads the ciphertext when the row has one. Otherwise it reads the token column, so callbacks parked before this change can still be revealed.
    • It returns 409 callback_url_unrecoverable only for a row with neither form. No version up to this one writes such a row.
  • Run page. Members with runs.control get a "Reveal URL" button, and everyone else is told who can reveal it. The panel is keyed by the wait, so a revealed URL never outlives its wait. When an API from before this change answers, the page shows the URL that API projects, as it always did.

  • Storage: the expand step of taking the token out of plaintext. Migration 0248_workflow_callback_token_digest.sql is additive:

    • It adds token_digest, the lowercase hex SHA-256, backfilled for every Callback row that holds a minted token.
    • It adds token_ciphertext, AES-GCM under the environment-held Variables:MasterKey. Data Protection is deliberately not used, because its key ring lives in the same database. SQL cannot reach the key, so only waits minted by this version get a ciphertext.
    • It adds a partial index on token_digest for pending Callback waits.
    • The token column keeps the plaintext token, and new waits still write it there, together with the digest and the ciphertext.
  • Resume. A resume matches the digest of the presented value, or matches a minted-shape value against token. The stored digest is never a key:

    • The previous build compares the presented value with token, which never holds a digest.
    • This build hashes whatever is presented.
    • Both builds' routes admit only 32 characters.
  • Not delivered here: at-rest secrecy of callback tokens. The plaintext stays in token for this release, exactly as on main today, so that previous-version pods and a rollback keep working. The contract step below removes it.

Deploy and rollback

  • Both images run DbUp at startup (Program.cs), so 0248 runs on the first new pod of either role to boot.
    • It adds two nullable columns with no default, so the ALTERs only touch the catalog.
    • It runs an UPDATE that fills only a NULL digest.
    • It builds the partial index, which holds a short write lock on workflow_run_wait while it scans the table once.
    • Running it twice changes nothing.
  • Any rollout order is safe, including pod by pod.
    • Previous-version API pods keep resuming every issued URL, including URLs this version minted, and refuse the digest.
    • A previous-version worker that parks a callback after 0248 writes a row with only token. This version resumes it and reveals it from that column.
  • The SPA and the API can be deployed in either order.
    • An older SPA in front of this API shows the placeholder URL for a Callback wait. Reloading the page loads the new SPA.
    • This SPA in front of an older API shows that API's projected URL.
  • A rollback to the previous build loses no data and strands no URL.
    • Every callback URL keeps resuming, including those this version minted.
    • The previous build ignores the two new columns.
    • Rolling forward again works.
    • While the previous build serves, its run page again shows the token to every member. That is the exposure this PR closes, so the Viewer fix holds only while this version's API serves.

TODO — follow-up contract release (needs migration 0250+)

This release keeps the plaintext token in workflow_run_wait.token. A later release must blank it, and only under these conditions:

  • Precondition. Ship it only once no build older than this one can run, including as a rollback target. After it, rolling back past this version strands every callback URL, because older builds look callbacks up only by token. Its PR must say "roll back only to this version or later".
  • Backfill token_digest where it is still NULL. These are Callback rows that previous-version pods parked after 0248 ran.
  • Seal token_ciphertext in the application for Callback rows that lack it. SQL cannot reach Variables:MasterKey.
  • Then set token = '' for Callback rows and stop writing the plaintext in WorkflowEngine (Token = bearerToken?.Token ?? correlationToken).
  • Never write a digest, the projection placeholder, or any other value a callback route can carry into token. Every build before the contract step compares whatever is presented with that column.
  • Keep resume-by-digest and reveal-from-ciphertext as they are. This version already handles blanked rows: A_url_whose_token_a_later_release_blanked_resumes_by_its_digest_and_reveals_from_its_ciphertext. A row blanked without a ciphertext still resumes but returns 409 on reveal.
  • Flip the assertions that pin the plaintext:
    • wait.Token.ShouldBe(token) in CallbackFlowTests;
    • storedToken.ShouldBe(token) in CallbackUrlRevealE2ETests;
    • wait.Token.ShouldBe(issued, …) in CallbackTokenDigestMigrationTests.

Test plan

  • Unit: WorkflowCallbackTokenSealerTests and WorkflowWaitKindsTests (60/60).
    • A minted token has the 32-hex shape and three stored forms.
    • The digest equals what 0248's SQL writes.
    • Only a minted-shape value is looked up in the plaintext column. A digest, an empty string and other shapes are not.
    • A reveal opens the ciphertext, or else the plaintext. It refuses a row with neither.
    • A ciphertext opens only under its own key.
    • The placeholder is pinned: it is decodable by every SPA, is not 32 characters, and is not hex.
  • Unit: full suite, 13683 passed and 1 skipped.
  • Integration: CallbackTokenDigestMigrationTests. The previous-version pod is simulated by the previous build's lookup run verbatim as SQL (token = @presented AND status = 'Pending' AND wait_kind = 'Callback'), followed by its resume.
    • Replaying 0248 over previous-version rows adds the digest, leaves every token unchanged and seals nothing. A second run changes nothing.
    • A URL issued before 0248 resumes on this version and can be revealed.
    • A previous-version pod resumes a URL issued before 0248 and refuses its digest.
    • After a rollback, the previous build resumes a URL this version minted and refuses its digest.
    • A row a previous-version pod parks after 0248 resumes and reveals.
    • A row whose token a later release blanked resumes by digest and reveals from its ciphertext.
    • A row with neither form still resumes and returns 409 on reveal.
  • Integration: CallbackFlowTests, ParallelWaitResumeScopeTests, BearerRouteLoggingTests and FailuresAreAlwaysLoggedTests. With the class above, 48/48.
    • The stored digest presented as a token does not resume.
    • Reveal returns 404 for another team's run and for a resolved wait.
    • A matched bearer route logs its template.
    • On a fallback, a 405, or before routing, the token segment is masked.
    • A non-bearer route is left raw.
    • The SPA routes equal the link paths.
  • E2E: CallbackUrlRevealE2ETests, BearerUrlTokenLogE2ETests and WebhookOutsiderHardeningE2ETests (25/25).
    • A Viewer sees the placeholder on both reads and gets 403 on reveal. A Member gets 200, and the revealed URL resumes the run anonymously.
    • The placeholder URL, a guessed token and the stored digest all return 404.
    • Nothing the database holds, including its own Data Protection key ring, opens the ciphertext.
    • No token reaches any log event, on success or failure, for wrong methods, for SPA links sent to the API, or for the first request to a fresh Production host.
  • Frontend, under Node 22:
    • SuspendedPanel.test.tsx covers reveal gated by runs.control, the panel reset on a new wait, and showing an older API's projected URL.
    • workflowRunPendingWaitApi.test.ts checks that the placeholder passes the decoder.
    • static-hosting.test.ts covers the nginx masks.
    • pnpm lint has 0 errors. tsc --noEmit -p tsconfig.app.json passes. The full vitest suite passes (2542/2542). pnpm build passes.
  • frontend/docker/e2e/run.sh: check the served nginx access log masks the token. This needs Docker.
  • Staging: roll pods in any order. Confirm that a callback URL issued before the deploy resumes the run on both old and new API pods, and that a Viewer's run page shows no URL. Confirm that the API's access log and Seq show {token} for invite, reset and callback paths.

Invitation, password-reset and workflow-callback tokens travel in URL
paths, and every request wrote its raw path to the console and Seq:
Serilog's completion line, the hosting scope's RequestPath on every
event, and GlobalExceptionFilter. The SPA's nginx logged the request
line and the Referer, which a page opened at /invite/<token> sends with
every asset. Anyone who could read logs could join a team, reset a
password or resume a run. A route that binds a `token` parameter is now
logged by its template (/api/invitations/{token}/accept), read live
from the first middleware. Where no bearer route answers -- a GET or
HEAD on a callback URL, a wrong method, the SPA's invite or reset link
opened on the API, any line logged before routing such as the https
redirect's port warning -- a path with a bearer route's shape is logged
with that segment as {token}. nginx logs {token} in place of the
segment. URL shapes are unchanged.

The callback-wait token was projected to every team member on run
detail and pending-wait, so a Viewer could resume a run anonymously
with a body of their choosing, bypassing runs.control. Both reads now
carry a fixed placeholder in its place -- not null, which the decoder
every earlier SPA ships refuses -- and the token is handed out only by
RevealWorkflowCallbackUrlCommand (runs.control, logged as a reveal).
The run page still shows a token that an API from before the reveal
projects, so the page works whichever role is deployed first.

Migration 0248 is the expand step of taking the token out of plaintext.
It adds token_digest, backfilled for every minted Callback token, and
token_ciphertext, an AES-GCM copy under the environment-held variable
master key for the reveal -- not Data Protection, whose key ring sits
in the same database. The token column keeps the token, and new waits
still write it, because a pod on the previous build looks a callback up
by comparing that column with what was presented. Hashing it in place
made previous-version API pods answer 404 to every issued URL, made
them accept the stored digest -- which the old UI shows every member --
as a working token, and made a rollback strand every URL. Now any
rollout order and a rollback keep every URL resuming, and the digest is
a key nowhere. This version resumes by digest, or by the token column
for rows the previous build wrote, so a later release can blank that
column once no older pod can run.

The run page keys the callback panel by its wait, so a revealed URL
never outlives the wait it belongs to.
@ppXD
ppXD force-pushed the fix/keep-bearer-url-tokens-out-of-logs-and-views branch from 7b289b0 to c115826 Compare October 13, 2026 07:38
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