feat(tools): let a product say what its capability bearer is bound to - #367
Conversation
authenticateToolRequest required the bearer to verify against the USER header. Since agent-interface 0.38 a credential may reach a profile only as a reference the sandbox resolves from the box environment, and that environment is workspace-wide and written once at box creation — so a per-user token cannot be delivered to a turn at all. Two apps on this shell are blocked on exactly that, and a third works only by putting the workspace id in the user header, which misattributes every record its tools write. subject: 'workspaceId' verifies the bearer against the workspace instead. Default stays 'userId', so an existing caller is byte-unchanged. This does not collapse the identity. The user header stays required, stays server-set, and stays on ctx, because downstream domain code attributes work to it. Only "may this caller act at all" moves to the workspace — which is what a box shared by every member of that workspace already implies. A per-user token written into that environment was never per-user: every member's turns read the same value. Order matters and is deliberate: a workspace-bound request with no workspace header is refused BEFORE verification, since verifying against an absent subject is not a check, and a product whose verify is lenient about an empty subject would otherwise authenticate anything.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 519f15b9
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-08-01T07:28:10Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 2 (1 medium-concern, 1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 64.5s (2 bridge agents) |
| Total | 64.5s |
💰 Value — sound-with-nits
Adds an opt-in, default-preserving subject option so a product can verify the bearer against the workspace instead of the user — a coherent, in-grain fix for real fleet breakage; only nit is it isn't plumbed through the high-level one-liner wrapper.
- What it does: Adds
subject?: 'userId' | 'workspaceId'toAuthenticateOptions(default'userId'). When set to'workspaceId',authenticateToolRequest(src/tools/auth.ts:83-87) verifies the bearer against the workspace header value instead of the user header, while still requiring the user header and returning it unchanged onctx. A workspace-bound request with no workspace header is refused 400 before - Goals it achieves: Lets a product whose capability token must live in the box environment — workspace-wide and written once at box creation (per AGENTS.md agent-interface 0.38 model) — verify against the only subject that token can actually be bound to, instead of being forced to either misattribute identity (gtm-agent's workspace-in-user-header trick) or break entirely (creative-agent, relationships-agent). Makes t
- Assessment: Good change, in the grain of the codebase. (1) Default-preserving: every existing caller is byte-identical, guarded by a test at auth.test.ts:24-30. (2) Doesn't collapse identity —
ctx.userIdstays required, server-set, and returned (auth.ts:80,90-93), correctly refusing the gtm-agent-style workaround the PR body calls out. (3) The fail-closed ordering (400 on missing workspace BEFORE verify) is - Better / existing approach: none at the primitive layer — this is the right approach. One plenum gap noted below.
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound-with-nits
A correct, default-preserving seam letting a product bind the capability bearer to the workspace, but the option is added only to the low-level authenticateToolRequest and is NOT forwarded by the high-level handleAppToolRequest one-liner the named consumers actually use.
- Integration: authenticateToolRequest is exported (src/tools/index.ts:11) and the lone internal caller is handleAppToolRequest (src/tools/http.ts:28). That caller constructs the auth opts by hand — { verifyToken: opts.verifyToken, headerNames: opts.headerNames } — and HandleToolRequestOptions (src/tools/http.ts:7-16) has no
subjectfield, so the new option is silently dropped on the only path products reach. - Fit with existing patterns: Fits the established seam pattern exactly. The package's invariant (AGENTS.md:16, README.md:119) is that domain/identity behavior enters through typed callbacks on AuthenticateOptions — verifyToken is the canonical example. Adding a sibling
subjectdiscriminator on the same options object, defaulting to the prior behavior, is precisely in-grain. It does not compete with any existing capability ( - Real-world viability: Robust on the paths tested. Ordering is fail-closed and correct: userId+bearer required first (auth.ts:80), then the chosen subject must be non-empty before verify runs (auth.ts:84, so a lenient verifyToken can't authenticate an absent workspace), then verify, then workspaceId still required (auth.ts:90). The subject selector
opts.subject === 'workspaceId' ? workspaceId : userIdis safe under th - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 subject isn't threaded through the high-level handleAppToolRequest wrapper or the /tools barrel [maintenance] ``
src/tools/http.ts:7-16 (HandleToolRequestOptions) exposes
verifyTokenbut NOT the newsubject, and http.ts:28 calls authenticateToolRequest without forwarding it — so a product using the advertised one-liner ('A product's route file becomes a one-liner: handleAppToolRequest', http.ts:23) CANNOT opt into workspace-bound bearer without dropping to the primitive directly.CapabilitySubjectis also not re-exported from src/tools/index.ts (onlyAuthenticateOptionsis, index.ts:15), so a produ
🎯 Usefulness Audit
🟠 subject is not reachable from handleAppToolRequest, the surface the stated consumers use [ergonomics] ``
AuthenticateOptions gained
subject(src/tools/auth.ts:52) but HandleToolRequestOptions (src/tools/http.ts:7-16) did not, and handleAppToolRequest forwards only verifyToken+headerNames to authenticateToolRequest (src/tools/http.ts:28) — confirmed by rg returning zerosubjecthits across the tools entry points. The PR body's own framing — 'a product's route file becomes a one-liner: handleAppToolRequest(request, cfg)' and the three blocked apps all wire that shape (docs/agent-shell-consolidati
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
✅ No Blockers —
|
| glm | deepseek | deepseek-flash | aggregate | |
|---|---|---|---|---|
| Readiness | 92 | 89 | 79 | 79 |
| Confidence | 65 | 65 | 65 | 65 |
| Correctness | 92 | 89 | 79 | 79 |
| Security | 92 | 89 | 79 | 79 |
| Testing | 92 | 89 | 79 | 79 |
| Architecture | 92 | 89 | 79 | 79 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.
🟠 MEDIUM Workspace-bound mode leaves ctx.userId unverified — identity guarantee collapses — src/tools/auth.ts
In
subject: 'workspaceId'mode,verifyToken(subject, bearer)(line 87) receives ONLY the workspaceId; theuserIdreturned on ctx (line 93) comes straight from a request header that the MCP builder emits as public profile config (mcp.ts:198defineAgentProfilePublicConfig). The bearer proves workspace membership, not user identity, and the token lives in the workspace-wide box env readable by every member. So any agent in the box can present any userId value and still pass auth — cross-user attribution inside a workspace is now forgeable, whereas the previou
🟡 LOW Test 'rejects a token minted for a different user' does not isolate the subject — src/tools/auth.test.ts
With headers user-a/workspace-a and
verifyToken = (subject) => subject === 'user-b', the assertion (401) passes whether the code verifies against userId ('user-a' ≠ 'user-b') OR wrongly against workspaceId ('workspace-a' ≠ 'user-b'). The subject selection is actually pinned by the first test ('verifies against the user by default' assertstoHaveBeenCalledWith('user-a','tok')), so this is a redundancy nit rather than a coverage hole — but worth tightening by assertingverifyToken's argument here too, per the repo's 'prove a test can fail' bar.
🟡 LOW Higher-level HTTP route wrapper doesn't forward new subject option — src/tools/auth.ts
handleAppToolRequestinsrc/tools/http.ts:28callsauthenticateToolRequestbut itsHandleToolRequestOptionsinterface (line 7) has nosubjectfield, and line 28 only passesverifyToken+headerNames. Products that usehandleAppToolRequestas their one-line route handler cannot use workspace-bound tokens without dropping down to the lower-levelauthenticateToolRequestcall. Thesubjectoption is additive (defaults to'userId', so nothing breaks), but the higher-level API is incomplete: addsubject?: CapabilitySubjectto `HandleToolRequestOpti
🟡 LOW Two distinct control-flow branches emit an identical 400 'Missing workspace context' body — src/tools/auth.ts
Lines 84-86 (subject absent before verify, subject='workspaceId' path) and lines 90-92 (workspaceId absent after verify, default path) both return
Response.json({ error: 'Missing workspace context' }, { status: 400 }). Same status, same body, two different causes. Impact is observability only — a product debugging a 400 cannot tell from the response alone whether the request failed the pre-verify subject-presence gate or the post-verify workspace-presence gate. Fix: distinguish the bodies (e.g.{ error: 'Missing workspace subject' }vs `{ error: 'Missi
🟡 LOW CapabilitySubject type not re-exported from tools barrel — src/tools/auth.ts
CapabilitySubjectis exported from./authbut not re-exported fromsrc/tools/index.ts. Consumers writingsubject: 'workspaceId'work fine via literal inference sinceAuthenticateOptionsIS exported and carries the type. Consumers that need a namedCapabilitySubjectvariable must deep-import from@tangle-network/agent-app/tools/auth. This matches the package's subpath-import convention but is worth noting as a discoverability gap.
🟡 LOW subject is not plumbed through handleAppToolRequest — the canonical app-tool path cannot use the new capability — src/tools/auth.ts
The new
subjectoption is only reachable by callingauthenticateToolRequestdirectly. The in-repo canonical consumerhandleAppToolRequest(src/tools/http.ts:28) calls it as{ verifyToken, headerNames }andHandleToolRequestOptionshas nosubjectfield, so the documented one-liner route pattern (export const action = ({request}) => handleAppToolRequest(request, cfg)) can never use workspace-bound tokens — the exact case this PR exists for. A product must bypass the standard route factory to adopt the fix. Fix: addsubjecttoHandleToolRequestOptionsand forward it through http.ts (also fixes the now-stale http.ts:11 doc 'token belongs to the header user').
tangletools · 2026-08-01T07:31:27Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 6 non-blocking findings — 519f15b9
Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-08-01T07:31:27Z · immutable trace
Two apps on this shell cannot complete the agent-interface 0.40 migration, and both are stuck on the same line in this package.
authenticateToolRequesthardcodes the bearer to verify against the user header:Since 0.38 a credential reaches a profile only as a reference the sandbox resolves from the box environment — and that environment is workspace-wide, written once at box creation. So a per-user token cannot be delivered to a turn at all. The shell offered no way to express the only token that CAN live there.
What that costs today, measured:
Invalid capability tokenThat third row is the tell. It works only because gtm does not use
ctx.userIdfor domain work. creative does —auth.ctxflows intospec.call(raw, auth.ctx, …)— so the same trick there would misattribute every proposal to a workspace id.The change:
Default unchanged, so every existing caller is byte-identical.
It does not collapse the identity. The user header stays required, stays server-set, stays on
ctx. Only "may this caller act at all" moves to the workspace — which is what a box shared by every member already implies. Worth stating plainly: a per-user token written into that box environment was never per-user; every member's turns read the same value. Naming the workspace as the subject makes the scope honest as well as usable.One ordering decision worth reviewing. A workspace-bound request with no workspace header is refused with 400 before
verifyTokenruns. Verifying against an absent subject is not a check — a product whose verify is lenient about an empty string would otherwise authenticate anything. A test assertsverifyTokenwas not called.Tests — 11, covering both subjects symmetrically: correct subject passed to verify, wrong-subject rejection for each, missing bearer, missing user header even when the bearer is not bound to it, product header names, and
ctxstill carrying the real user under workspace binding. Calibrated: ignoring the option fails 3.tsc --noEmit→ 0.src/tools/green. The 1 failure intests/app-auth.test.tsisbetter-sqlite3native bindings missing on my machine (pre-existing, CI builds them).