Turn the security invariants into guard tests that fail when a new route, tool, or connector forgets them #306

Activity

1. serge-ivo

   added

   backendBackend / Worker / API workBackend / Worker / API work

   securitySecurity hardening / audit findingSecurity hardening / audit finding

   on Aug 6, 2026
2. serge-ivo

   added a commit that references this issue on Aug 6, 2026

   test(security): make the invariants fail the build when the next call…

   ...

   ff372a1
3. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Shipped in ff372a1.

   Six guards in workers/api/src/lib/security-invariants.test.ts, over a pure lexer in lib/source-guard.ts (comments, string literals and regex literals blanked, line numbers preserved) with its own 23 unit tests:

   1. Suspension reaches every authenticated surface — a file that calls verifySession must also call isSuspended, with two documented non-authorizing exceptions (error attribution in index.ts, bucket keying in rate-limit.ts), each ratcheted so a stale exemption fails.
   2. The connector layer never bypasses safeFetch — no bare fetch( under lib/connectors/**, lib/steps.ts, lib/tools.ts, agent-do-knowledge.ts.
   3. Mutating registry tools pass the write-consent gate — a connector tool whose name reads as mutating is scope:"write"; a tool that lets the caller choose the HTTP method cannot be scope:"read"; a write-scoped tool names its connector.
   4. Credentials have one envelope scheme — the credential tables are derived from the migrations (a ciphertext column + a wrapped-DEK column), and every INSERT/UPDATE writer must import lib/crypto.ts; crypto.subtle.{wrapKey,unwrapKey,encrypt,decrypt} exists in exactly two named places.
   5. The untrusted fence is not re-implemented — the marker string lives only in lib/untrusted-fence.ts.
   6. Destructiveness is judged on our own name — no code reads destructiveHint/readOnlyHint/idempotentHint/openWorldHint, and isDestructiveToolName is still what the grant check calls.

   Mutation-tested: 13 mutations, one per assertion, each confirmed red then reverted. Two first drafts stayed green under mutation and were wrong — one tested for the isSuspended identifier (which the import line satisfies on its own), the other scanned comment-stripped code for SQL that only ever lives inside string literals and had been finding zero writers. Both now carry explicit non-vacuity assertions.

   Violations found and fixed here

   • routes/public.ts — BOTH authenticated routes (POST /v1/public/webhook/:instanceId/ingest, POST /v1/public/agents/:id/import-gdoc) verified sessions inline, so the [Admin] MCP repo/scaffold tools bypass the requireUser suspension gate #273 suspension fix never reached them. Now requireUser. Regression test added.
   • routes/agents.ts — same on GET /v1/agents/:id; auth is optional there so it applies isSuspended by hand, like the WS upgrade. Two regression tests added.
   • connectorClient.fetch() — its URL is the caller's, so it was a standing SSRF bypass carrying the owner's minted Bearer. Now safeFetch. Unused today, which is the only reason it was not already one.

   Violations found and filed, not fixed

   • http_request is scope:"read" but can POST/PUT/DELETE with the owner's key — it never reaches the write-consent gate #307 — http_request is scope:"read" while accepting method:"DELETE" with the owner's vault key, so it never reaches the write-consent gate. A scope flip would break every read-only GET in the pipelines; it needs a per-call gate. Recorded as a ratcheted exception: guard 3 fails on a second such tool and fails when this one is fixed.
   • fetch_url / http_request / web_search return remote text unfenced — the untrusted fence covers RAG and MCP resources only #308 — fetch_url / http_request / web_search return remote text unfenced; the fence covers RAG and MCP resources only.

   CI on ff372a1 passed every step through pnpm test (4211 tests). It failed on three console e2e specs that were already red three commits earlier — tracked as #309.
4. serge-ivo

   closed this as completedon Aug 6, 2026
5. serge-ivo

   added 2 commits that reference this issue on Aug 6, 2026

   fix(security): gate the mutating http call, fence the remote text it …

   ...

   54b2228

   fix(security): give raw HTML one door and one mint, and count both (#297

   ...

   63aee0f