fix(security): stop agent runtime tokens enumerating private pods - #793
Merged
Conversation
GET /api/agents/runtime/pods filtered only on pod TYPE — excluding DM-shaped pods — and treated that as a visibility rule. It is not. Every other private pod stayed enumerable by ANY agent runtime token, including one scoped to a single pod. Verified live against production with a token for an agent installed in exactly one public pod: 20 pods returned (16 chat, 4 team), including multiple users' private "My Workspace" rows and a private team pod — and 5 carried `latestSummary`, i.e. generated summaries of conversations the caller had no access to. Pod names and conversation content, to any agent. #781 landed the DM-type exclusion, which is real and is kept. It was mistaken — by three separate reviewers including me — for a fix to the wider disclosure. An agent may now see a pod when it is either publicly listed or one the agent is installed in. Reuses COMMUNITY_LISTING_QUERY from services/podListing.ts rather than restating the rule, so this route cannot drift from the human-facing Discover surface the way it just did. Found by the sprint agents. Their sharper observation is worth keeping: three reviewers asserted this route's state without running it, because inferring feels like finishing. The check took thirty seconds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
samxu01
pushed a commit
that referenced
this pull request
Aug 2, 2026
ADR-017 gains "the channel is bidirectional" — the missing inverse of everything else in it. Routing so far is agent -> human; the 2026-08-01 incident is system -> agent: the disclosure was fixed, merged, deployed and verified, and the pod was never told, so four agents kept planning around an exposure closed an hour earlier. Recorded as a principle, not a mechanism: an invalidation is not an escalation (the trigger is "a fact you relied on changed", which is observed class 3 pointed the other way — one mechanism, two directions), and its cost is silent and asymmetric (a missed escalation stalls one agent visibly; a missed invalidation leaves every agent producing correct-looking work over a dead premise). Explicitly do NOT build a subscription system: n=1 is not a mandate for a dependency graph. Notes that `basis` is already this signal read from the other end. ADR-016's enforcement-gap table updated against origin/main: #793 closed the gap by composing COMMONLY_LISTING_QUERY with the caller's authorized pods. Residual divergence recorded — it uses the flags-only fragment, not communityDiscoverQuery, so invite-only listed pods appear on the agent surface while excluded from the human one. Not a leak (all publicRead), but the route's own comment claims it cannot drift, and it still differs. AX audit: entry 5 (nothing tells an agent its premise expired), entry 1 marked closed by #793. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 2, 2026
samxu01
pushed a commit
that referenced
this pull request
Aug 2, 2026
ux-lead's design call on the #793 residual, replacing the either/or I left open. The agent discovery surface adopts communityDiscoverQuery rather than the flags-only fragment: a comment asserting parity over a query that diverges is a phantom-contract seedling watering itself. The "agents should see request-access-able rooms" case is real but is the H5 case, and the 2026-07-29 dead-end ruling applies to agents equally — a discoverable row with no available action yields a 403 whose only use is relaying confusion, and that 403 is not machine-readable as "requestable later" either. Rule recorded: divergence between the human and agent visibility surfaces must be a decision with an affordance attached, never a side effect of which query constant a route imported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
samxu01
pushed a commit
that referenced
this pull request
Aug 2, 2026
Supersedes the comment-only change on this branch. sprint-review out-argued the
position it documented, so the code moved instead of the comment.
My justification for the divergence was that an agent should see a room it
could ask to join. That does not hold: H5 request-access does not exist, so an
agent shown an invite-only row can neither join nor request access. It is a
dead end — the same reasoning that excluded those rows from human Discover.
The argument was not wrong, it was premature; it becomes correct when H5 ships
and the row acquires a verb. Recorded as the revisit trigger.
Adopts joinPolicy: { $ne: 'invite-only' } only. Deliberately does NOT adopt
communityDiscoverQuery wholesale: its members: { $ne: callerId } clause exists
because human Discover means 'find something new', while this route means 'what
may I see' — adopting it would delete with one hand what the $or branch adds
with the other. Both halves now have a test.
This also makes #793's comment true rather than aspirational, which is the
point: AX entry 1 is about exactly the class of comment that asserts an
invariant the code does not hold.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lilyshen0722
added a commit
that referenced
this pull request
Aug 4, 2026
* docs(security): state the agent-discovery asymmetry instead of overclaiming it My #793 comment said the route 'cannot drift from the human-facing Discover surface again'. It composes COMMUNITY_LISTING_QUERY (flags only), not communityDiscoverQuery, which additionally excludes invite-only pods and pods the caller already belongs to — so it still differs by one clause. Nothing private is exposed: every branch is either publicRead or the caller's own installation. The asymmetry is defensible and I am keeping it — an agent should be able to see a room it could ask to join. What was wrong was the comment promising more than the code delivered. Caught by pod-architect while verifying #793 before writing about it, which is the same phantom-contract class this sprint has been cataloguing: a docstring asserting an invariant the code does not hold. Mine, this time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(security): exclude invite-only pods from agent discovery Supersedes the comment-only change on this branch. sprint-review out-argued the position it documented, so the code moved instead of the comment. My justification for the divergence was that an agent should see a room it could ask to join. That does not hold: H5 request-access does not exist, so an agent shown an invite-only row can neither join nor request access. It is a dead end — the same reasoning that excluded those rows from human Discover. The argument was not wrong, it was premature; it becomes correct when H5 ships and the row acquires a verb. Recorded as the revisit trigger. Adopts joinPolicy: { $ne: 'invite-only' } only. Deliberately does NOT adopt communityDiscoverQuery wholesale: its members: { $ne: callerId } clause exists because human Discover means 'find something new', while this route means 'what may I see' — adopting it would delete with one hand what the $or branch adds with the other. Both halves now have a test. This also makes #793's comment true rather than aspirational, which is the point: AX entry 1 is about exactly the class of comment that asserts an invariant the code does not hold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: share directly joinable discovery query --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
lilyshen0722
added a commit
that referenced
this pull request
Aug 4, 2026
* docs: ADR-016 + ADR-017 full drafts, reviewer checklist Replaces the ADR-016/017 stubs merged in #775 with the full drafts, and adds the incident-derived reviewer checklist assembled during milestone #11. ADR-016 (pod model and visibility) — kind x visibility tier x joinPolicy over the existing flags, no schema change. 7-state reachable enumeration, migration for the unrepresentable state, and an Enforcement gaps section verified against origin/main: 4 of 5 read surfaces consult the visibility tier; GET /api/agents/runtime/pods does not, and still returns latestSummary for non-member pods. ADR-017 (attention routing) — judge divergence + static irreversibility feeds, escalation envelope with required typed evidence, needs-you card with four lifecycle-visible faces, in-pod first. Budget sized against EEMUA/ISA-18.2 rates (source-verified, with the transfer caveat stated). docs/development/review-checklist.md — 13 reviewer rules, each carrying the incident that earned it. Both ADRs are Proposed, not Accepted; ratification is Sam's. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ADR-016 — fix agent-admin reachable states, align joinPolicy migration Both from sprint-review's #792 pass, re-verified against origin/main: agent-admin is in NON_LISTABLE_POD_TYPES and refused by both visibility writers, so calling it a plain room overstated the enumeration — states 3-6 are unreachable for it. Adds a third derived kind (admin-room): terminally private like a DM, for a different reason (listability, not cardinality). Total reachable states 7 -> 8. Migration step 3 normalized null joinPolicy to invite-only while the schema and creation path default to 'open'. Corrected to 'open': the narrowing lives in the tier (self-joinable <=> community AND open), so a private pod with joinPolicy 'open' is still not self-joinable, and the migration should not be the one writer that disagrees with the schema. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ADR-017 — authority boundary as primary trigger, per corpus labelling sprint-review labelled this pod's own unattended run (238 messages, four agents, four days): 15 warranted interrupting a human (6.3%), sustained 0.16/hr, peak 31/10min raw vs 5/10min filtered. The finding that changed the design: "escalate on irreversibility" caught zero of fifteen. The dominant class (8/15) is not misbehaviour at all — an agent correctly finishing and hitting a wall only the human can pass. So the primary trigger is structural, not behavioural: has this agent reached a boundary it cannot cross? The system already knows, because the boundary is the permission set — a query, not an inference, and it covers the largest class with no model. - new Layer 0 recording the corpus, the four observed classes ranked, and the caveat (n=15, one pod, one decision-maker) - authority boundary promoted to primary feed, with evidence typed as { boundary, artifact, availableTransitions } so cards name the decision - irreversibility kept as a safety net, with its zero-fire record stated: unbounded false-negative cost, bounded false-positive cost - envelope feed/class enums updated to the observed taxonomy Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: AX audit log + sharpen ADR-017's irreversibility caveat The zero-irreversibility caveat now states the labeller's own distinction: this corpus had no delete/spend/send permissions in play, so the zero is strong evidence that irreversibility is the wrong PRIMARY trigger and weak evidence about its firing rate where agents hold destructive capability. The zero measures the permission profile, not the mechanism's worth. Adds docs/development/agent-experience-audit.md — the AX findings Sam asked the sprint agents for, which were accumulating only in pod chat. Four entries: the docstring is the interface; permitted verdicts are undiscoverable until refused; silent success and silent failure look identical; pod prose is not delivery. Recurring shape: an agent's model of the system comes almost entirely from names, docstrings and error messages. Where those lie or stay silent, it forms a confident wrong model — with no visual channel to correct it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: bidirectional attention channel, #793 verified, AX entry 5 ADR-017 gains "the channel is bidirectional" — the missing inverse of everything else in it. Routing so far is agent -> human; the 2026-08-01 incident is system -> agent: the disclosure was fixed, merged, deployed and verified, and the pod was never told, so four agents kept planning around an exposure closed an hour earlier. Recorded as a principle, not a mechanism: an invalidation is not an escalation (the trigger is "a fact you relied on changed", which is observed class 3 pointed the other way — one mechanism, two directions), and its cost is silent and asymmetric (a missed escalation stalls one agent visibly; a missed invalidation leaves every agent producing correct-looking work over a dead premise). Explicitly do NOT build a subscription system: n=1 is not a mandate for a dependency graph. Notes that `basis` is already this signal read from the other end. ADR-016's enforcement-gap table updated against origin/main: #793 closed the gap by composing COMMONLY_LISTING_QUERY with the caller's authorized pods. Residual divergence recorded — it uses the flags-only fragment, not communityDiscoverQuery, so invite-only listed pods appear on the agent surface while excluded from the human one. Not a leak (all publicRead), but the route's own comment claims it cannot drift, and it still differs. AX audit: entry 5 (nothing tells an agent its premise expired), entry 1 marked closed by #793. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ADR-016 — surface parity is the rule, H5 is the exception-point ux-lead's design call on the #793 residual, replacing the either/or I left open. The agent discovery surface adopts communityDiscoverQuery rather than the flags-only fragment: a comment asserting parity over a query that diverges is a phantom-contract seedling watering itself. The "agents should see request-access-able rooms" case is real but is the H5 case, and the 2026-07-29 dead-end ruling applies to agents equally — a discoverable row with no available action yields a 403 whose only use is relaying confusion, and that 403 is not machine-readable as "requestable later" either. Rule recorded: divergence between the human and agent visibility surfaces must be a decision with an affordance attached, never a side effect of which query constant a route imported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ADR-016 — parity is per-clause; "adopt the builder" was wrong sprint-review caught a defect in the rule committed one commit earlier. "Adopt communityDiscoverQuery on the agent route" treats three clauses as one decision; only the joinPolicy clause belongs there. - listing flags: shared (the visibility tier itself) - joinPolicy $ne invite-only: adopt — a row with no available action is a dead end for either reader, and the 403 isn't machine-readable as "requestable later" - members $ne callerId: never — the surfaces have different jobs, and it is subtly unsafe here: the route's second $or branch keys on installations, not membership, so a pod the agent is a member of without an active installation would be excluded by the clause and not restored by the branch Shared unit is therefore a fragment (flags + joinPolicy) that both surfaces compose, each adding its own caller clause — the same lesson as the original fragment/builder split, one level down. Urgency recorded as none: 3 community-listed pods, 0 invite-only, so the divergence is theoretical and a test would pass vacuously today. Revisit trigger is H5 landing, when joinPolicy drops from both surfaces together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ADR-017 — fix seven inconsistencies from the end-to-end read sprint-review's full pass (PR #792 review 7) found seven locally-correct edits that had stopped agreeing with each other. All were introduced by incremental rewrites and none were caught by incremental review. 1. Judge's class enum conflated two taxonomies. The judge cannot detect authority-boundary or deadlock — those come from the no-model query feed. Judge now emits divergenceClass (scope-expansion, target-change, abandonment, other); the envelope's top-level class carries the observed taxonomy, with a comment stating they are never merged. 2. "Both feeds" / "two feeds" -> three feeds, four routing inputs. 3. Ratification point 3 asked Sam to ratify a "four-class taxonomy" that no longer exists. Now names both taxonomies explicitly. 4. Override statistic appeared as both 49-96% (pre-verification) and 46-96% (verified). Unified on the verified figure. 5. "Two-tier staleness rule:" lead-in survived the unification that replaced it with one rule. 6. "Latest human word wins" contradicted "re-binding is ordinary agent behavior". The latter is correct and verified in code; corrected to ordering-not-identity, with an explicit warning never to write a human-only gate since no issuer field exists to build it on. 7. Deadlock listed under class-1 instances; it is class 4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ax): entry 6 — a documented call shape the tool cannot express The heartbeat instruction directs agents to append cycle takeaways via commonly_save_my_memory with a nested { append: { content } } payload. The deployed tool schema accepts only content (string) or entries (array) with additionalProperties: false, so no reachable argument shape produces it; all three forms 400 with the server naming a payload the tool cannot emit. `cycles` is also absent from the tool's own documented section list. Three surfaces describe the same capability differently — scheduler instruction, tool schema, server validator — and only the last is authoritative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ax): entry 7 — directives and arguments share one identity The orchestrating assistant posts under the operator's account, so "take #795 next" and "here is my read of the taint path" arrive in one voice. Agents defaulted to treating both as directives, which is correct when they cannot be told apart — and a technical claim propagated two review cycles unchecked, then landed misattributed in a PR approval that is now the durable record of a design choice. A directive should be followed; an argument should be checked. Identity is the only signal an agent has, so where one identity carries both, the weaker treatment wins by default. Interim protocol recorded pending per-seat identities (#791). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ax): entry 7 — correct its own byline, record the cascade The entry about misattribution-through-shared-identity was itself misattributed: filed crediting ux-lead, who declined it. The content came from the orchestrating assistant posting under the operator account (52211). sprint-review's log check found the full cascade: 52204 (operator account) -> credited to pod-architect -> declined -> re-credited in a PR approval because the declination never arrived -> refiled here against a third wrong seat. Four misattributions in one incident, among participants actively trying to attribute correctly, one inside the document describing the problem. Corrected, the entry is self-demonstrating rather than self-refuting: no amount of diligence substitutes for a distinguishable identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: land the four pointer edits the checklist header promised review-checklist.md opened with a "Lands with (pointer edits, same PR)" line naming four edits that were never in the diff — the doc introducing rule 7 (phantom cross-layer contract) shipped as one: a header promising that another file provides something, with nothing holding the two together. All four now exist: - docs/development/README.md — index rows for the checklist and the AX log - REVIEW.md — context-loading step 6 citing §7; two-checklists note above the author checklist, so the name collision is broken from both sides - CLAUDE.md — companion pointer beside the REVIEW.md required-reading line, plus an anchor for the AX audit - CLAUDE.md — the NO_REPLY correction. Main read "it will be sent verbatim", wrong since PR #785: a bare sentinel in substantive content is stripped as producer leakage; backticked/fenced mentions survive; suppression stays total-match. Verified against sanitizeAgentContent and its test file, not against the prose that described it. Also in the checklist: repair a mangled clause in rule 7, and record the outer-transport-fence mechanic that makes the §9 tests read correctly. AX entry 6 corrected. It concluded the cycles append was unreachable through the tool surface; commonly_log_cycle has owned that verb since ADR-012 Phase 4 (#308/#309), two months earlier. Re-probed both calls today — every fact in the entry holds, the conclusion did not. The real defect is narrower: the capability is owned by one tool and named by another, and the 400 names the payload it wants without naming the tool that can emit it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Live on production. Verified, not inferred.
GET /api/agents/runtime/podsfiltered only on pod type — excluding DM-shaped pods — and treated that as a visibility rule. It isn't.Measured with
hq-support's token, an agent installed in exactly one public pod:So: private pod names, plus generated summaries of conversations the caller has no access to, to any agent runtime token.
#781's DM exclusion is real and is kept — it removed the 10 one-to-one rooms. It was mistaken, by three separate reviewers including me, for a fix to the wider disclosure. It only ever hid pods that are private by type.
The fix: a pod is visible when it is either publicly listed or one the agent is installed in. Reuses
COMMUNITY_LISTING_QUERYfromservices/podListing.ts(added by #780) rather than restating the rule, so this route cannot drift from the human-facing Discover surface again — which is exactly how it got here.Test: 4 regression tests, proven load-bearing — 3 of 4 fail against the pre-fix query. Covers the visibility branches, that the DM type-guard survives, and that an agent with no installations degrades to public-listed-only rather than to an unfiltered find.
Credit and the lesson: found by the sprint agents. Their sharper observation is the one worth keeping — three reviewers asserted this route's state without running it, because inferring feels like finishing. The check took thirty seconds.