Skip to content

fix(harness): compare the /ingest bearer token in constant time - #658

Open
memosr wants to merge 1 commit into
sapiom:mainfrom
memosr:fix/harness-ingest-timing-safe-token
Open

fix(harness): compare the /ingest bearer token in constant time#658
memosr wants to merge 1 commit into
sapiom:mainfrom
memosr:fix/harness-ingest-timing-safe-token

Conversation

@memosr

@memosr memosr commented Aug 17, 2026

Copy link
Copy Markdown

Primary change type

  • Bug fix

Problem and motivation

POST /ingest checks the boot token with a plain string comparison:

const token = bearerToken(req.headers.authorization);
if (token !== deps.ingestToken) { ... }

Every other check of that same secret already runs in constant time:

  • createBootTokenMiddleware (server/auth.ts), which guards the whole /api surface
  • the WS upgrade path (server/events-ws.ts)

timingSafeEqualString even balances its length-mismatch branch on purpose, and its doc comment states why. /ingest is the one path left out, and deps.ingestToken is the same options.bootToken the other two protect.

The server binds to 127.0.0.1, so this is not remotely reachable. The change brings one path in line with the package's own established pattern.

Summary and scope

Use the existing helper. One import, one condition, no new dependency, and no behavior change for valid or invalid tokens.

Out of scope: the auth helper itself and the other two call sites are already correct and are not touched.

Related work

Related issue or discussion: N/A. See the Security section below — I would like maintainer guidance on whether this should have gone through the Security Policy instead.

Validation

pnpm --filter @sapiom/harness exec vitest run src/server   — pass (291/291)
pnpm --filter @sapiom/harness exec vitest run src/server/ingest.test.ts — pass (12/12)
pnpm --filter @sapiom/harness typecheck                    — pass
pnpm --filter @sapiom/harness lint                         — pass (0 errors)
pnpm build                                                 — pass

Tests and documentation

Tests: added a case for a request with no Authorization header at all, which was untested and is the path the ?? "" now covers. The existing wrong-token case already exercises a token of a different length.

Documentation: N/A, no user-facing behavior changed.

Compatibility and release impact

  • Breaking or externally visible changes: None. Valid tokens are still accepted and invalid ones still get 401.
  • Changeset: Added (patch, @sapiom/harness).

Security

  • I have not included secrets, credentials, private data, or unsanitized logs.
  • This pull request does not publicly disclose a suspected vulnerability.

I want to be straightforward about the unchecked box rather than quietly tick it. I judged this as hardening of a local-only path that simply missed the pattern already applied to /api and the WS upgrade, so I opened it directly. Re-reading CONTRIBUTING.md ("even if it looks like a bug fix"), I am no longer confident that was the right call.

Nothing here is novel: the fix is a one-line use of a helper that already exists in the repository for exactly this purpose, and the gap is visible to anyone reading the two files side by side. But if you consider this in scope for the Security Policy, say so and I will close this PR and send it to team [at] sapiom.ai instead. I will follow the private channel for anything less clear-cut in future.

AI assistance

  • I used AI assistance and have described it below.

I used Claude (Claude Code and the Claude app). Claude reviewed the package and reported that /ingest was the only boot-token check not using timingSafeEqualString; I confirmed this by reading auth.ts, events-ws.ts, ingest.ts and the three ingestToken: options.bootToken call sites in server/index.ts myself. I directed the fix and the added test case, and Claude applied them. I ran the full server suite and the type and lint checks and reviewed the diff before pushing.

Checklist

  • I read CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.
  • This pull request addresses one focused problem and contains no unrelated cleanup.
  • I added or updated tests, or explained above why tests are not applicable.
  • I ran the relevant build, typecheck, lint, and test commands, or explained any N/A checks above.
  • I updated documentation for user-facing changes, or marked it N/A above.
  • I added a Changeset for a published-package change, or explained why it is not applicable.
  • I can explain and maintain every submitted change, including any AI-assisted work.

Every other boot-token check uses timingSafeEqualString: createBootTokenMiddleware guards the whole /api surface, and the WS upgrade path guards events-ws. Its doc comment states the reason, and it even balances the length-mismatch branch so that case does not resolve faster.

POST /ingest was the one path left on a plain !== against the same secret. The server binds to 127.0.0.1, but that is precisely the threat model here: the harness runs on a machine where a coding agent executes arbitrary commands, so a local process that can reach /ingest but does not hold the token could recover it byte by byte and then use it against all of /api.

Use the existing helper. Also cover the missing-header case, which was untested.
@github-actions github-actions Bot added contribution: incomplete Required pull request information is incomplete or ambiguous contributor: external Pull request author does not have write, maintain, or admin access to sapiom-js needs-triage Awaiting maintainer review and classification review: manual External pull request requires maintainer review before automation size: small Review size is at most 100 changed lines area: studio Changes to Agent Studio or harness applications labels Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: studio Changes to Agent Studio or harness applications contribution: incomplete Required pull request information is incomplete or ambiguous contributor: external Pull request author does not have write, maintain, or admin access to sapiom-js needs-triage Awaiting maintainer review and classification review: manual External pull request requires maintainer review before automation size: small Review size is at most 100 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant