Skip to content

Commit 6542970

Browse files
committed
docs(consent), test(bounded-body): state the guard's real limit, pin the shape
Two round-3 audit findings, one accepted as a documentation fix and one as a test-instrument fix. A local process running as the user can mint its own GUI session — the dashboard bootstrap is served to any loopback GET — and can equally run `gh api -X PUT /user/starred/...` without involving the proxy at all. No check inside this process distinguishes that caller from the browser, because both hold every local credential. Claiming the endpoint is a technical barrier would be false, so the route comment and AGENTS.md now say what it actually does: it removes the casual path and the raw-token path, and the real boundary is the normative rule that an agent must not spend the user's identity by any mechanism. The unconditional session requirement stays; it is just no longer described as more than it is. The bounded-body growth counter caught an exact-fit reallocation mutation but not the per-chunk `Uint8Array[]` it replaced, because that implementation never increments the counter at all. The retained-object shape is now pinned structurally — no per-chunk collection, one geometric buffer — the same instrument this repository already uses for the relay retention rule. Driven red by restoring the array accumulator.
1 parent cec481c commit 6542970

3 files changed

Lines changed: 43 additions & 5 deletions

File tree

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,16 @@ with regression coverage in `tests/startup-prompt.test.ts`,
130130
another action that spends the user's identity, credits, or reputation, gate it
131131
the same way rather than relying on a prompt an agent can answer.
132132

133+
**Be clear about what that enforcement is and is not.** The management endpoint
134+
requires a dashboard session, which stops the casual path — an agent that would
135+
have POSTed there because the endpoint existed, and one holding only the admin
136+
token. It is not a technical barrier against a determined local agent: a process
137+
running as the user can mint its own session from the loopback dashboard
138+
bootstrap, and can skip the proxy entirely by running `gh` itself. Every local
139+
credential is equally reachable by both the browser and the agent, so no check
140+
inside this process can tell them apart. The real boundary is the rule above, and
141+
it binds you regardless of which mechanism is within reach.
142+
133143
## Commands
134144

135145
```bash

src/server/management/sidebar-routes.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@ function hasBrowserSessionEvidence(ctx: ManagementContext): boolean {
4747
// and the response names the one-line `gh` command to run by hand. That is the
4848
// correct trade — an endpoint reachable with a readable token cannot establish that
4949
// a human chose to spend their own GitHub identity.
50+
//
51+
// The honest limit of this guard: a local process running AS THE USER can mint its
52+
// own GUI session (the dashboard bootstrap is served to any loopback GET) and can
53+
// equally just run `gh api -X PUT /user/starred/...` itself, which needs no proxy at
54+
// all. No check inside this process can distinguish that caller from the browser,
55+
// because both hold every local credential. So this endpoint is not a technical
56+
// barrier against a determined local agent — it removes the CASUAL path (an agent
57+
// that would have POSTed here because the endpoint existed) and makes the refusal
58+
// legible. The actual boundary is normative and lives in AGENTS.md: an agent must
59+
// not spend the user's identity, whichever mechanism is at hand.
5060

5161
export async function handleSidebarRoutes(ctx: ManagementContext): Promise<Response | null> {
5262
const { req, url } = ctx;

tests/bounded-body.test.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,10 @@ describe("readBoundedResponseBody", () => {
165165
});
166166

167167
test("retention is logarithmic in the body, not linear in the chunk count", async () => {
168-
// The accumulator is the security property, and correctness cannot see it:
169-
// a per-chunk array reassembles identically while retaining one object per
170-
// transport chunk, which a fragmenting peer inflates far past the payload
171-
// ceiling. Growth count is the observable that separates the two — a single
172-
// geometric buffer doubles a handful of times regardless of fragmentation.
168+
// Growth accounting for the single buffer: it doubles a handful of times no
169+
// matter how the peer fragments the body. This catches an exact-fit
170+
// reallocation mutation; the per-chunk ARRAY shape is caught structurally in
171+
// the test below, because that implementation never touches this counter.
173172
const fine = Array.from({ length: 20_000 }, () => new Uint8Array([0x61]));
174173
await readBoundedResponseBody(responseFromChunks(...fine), { maxBytes: CUSTOM_CAP });
175174
const fineGrowths = boundedBodyBufferGrowthsForTests();
@@ -189,6 +188,25 @@ describe("readBoundedResponseBody", () => {
189188
await readBoundedResponseBody(responseFromChunks(...big), { maxBytes: CUSTOM_CAP });
190189
expect(boundedBodyBufferGrowthsForTests()).toBeLessThanOrEqual(4);
191190
});
191+
192+
test("the accumulator never retains one object per transport chunk", async () => {
193+
// The retained-object shape is the actual security property and no behavioral
194+
// assertion can see it: a `Uint8Array[]` of chunks reassembles byte-identically
195+
// while holding one reference per chunk, which a fragmenting peer inflates far
196+
// past the payload ceiling. It also never increments the growth counter above,
197+
// so that test alone cannot catch it. Pin the shape, the same instrument this
198+
// repository uses for the relay retention rule and the star-consent guard.
199+
const source = (await Bun.file(new URL("../src/lib/bounded-body.ts", import.meta.url)).text())
200+
.replace(/\/\*[\s\S]*?\*\//g, "")
201+
.replace(/(^|[^:])\/\/.*$/gm, "$1");
202+
203+
// No per-chunk collection: the reader must accumulate into one buffer.
204+
expect(source).not.toMatch(/chunks\s*\.\s*push\s*\(/);
205+
expect(source).not.toMatch(/const\s+chunks\s*:\s*Uint8Array\[\]/);
206+
// And that buffer must be the geometric one this module documents.
207+
expect(source).toMatch(/let\s+retained\s*=\s*new\s+Uint8Array\(/);
208+
expect(source).toMatch(/retained\.set\(value,\s*retainedBytes\)/);
209+
});
192210
});
193211

194212
test("parent abort rejects with the exact reason object", async () => {

0 commit comments

Comments
 (0)