Public: anyone can read this; the crew can change it.
The live outsider suite never reports ok when it skipped the owner-scoped checks #2894
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2879-the-live-outsider-suite-never-reports-ok → main 3 commits fork 4 threads · 4 unresolved
2 files
changed +48
−11
src/release/outsider.ts +16 −11
| @@ -94,8 +94,12 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 94 | 94 | const cookies = { cookie: session.headers.getSetCookie().map((c) => c.split(";")[0]!).join("; ") }; |
| 95 | 95 | if (!cookies.cookie.includes("forge_session=")) bad.push({ check: "sign in", why: `the fresh account got no session (${session.status})` }); |
| 96 | 96 | |
| 97 | // One MCP tools/call as the stranger: an error answer is a refusal. | |
| 98 | const mcp = async (tool: string, args: Json, as = token): Promise<{ error: boolean; text: string }> => { | |
| 97 | // One MCP tools/call as the stranger. Only an answer that names not-a-member, | |
| 98 | // unverified or forbidden counts as a refusal (an argument error is not one); | |
| 99 | // `hides` also accepts the not_found a door gives where hiding is the intent: | |
| 100 | // "repo" for a repository the stranger may not see, "tool" for the admin tool | |
| 101 | // `signup` (tools/list is checked to still name it, so a moved tool is red). | |
| 102 | const mcp = async (tool: string, args: Json, as = token, hides?: "repo" | "tool"): Promise<{ error: boolean; text: string }> => { | |
| 99 | 103 | const res = await doFetch(new URL("/mcp", root).href, { |
| 100 | 104 | method: "POST", |
| 101 | 105 | redirect: "manual", |
| @@ -107,8 +111,9 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 107 | 111 | try { |
| 108 | 112 | const result = (JSON.parse(text) as { result?: { isError?: boolean; content?: { text?: string }[] } }).result; |
| 109 | 113 | const said = result?.content?.[0]?.text ?? ""; |
| 110 | // A tool that does not exist refuses everything; that is a moved door, not a closed one. | |
| 111 | return { error: res.status === 200 && result?.isError === true && /^(not_member|not.a.member|unverified|forbidden|not_found|not_admin|admin_only|unauthorized)\b/i.test(said), text: said }; | |
| 114 | // A tool that does not exist refuses everything; that is a moved door, not a closed one | |
| 115 | // (only `signup`, whose presence tools/list proves, may answer no such tool). | |
| 116 | return { error: res.status === 200 && result?.isError === true && (/^(not_member|not.a.member|unverified|forbidden)\b/i.test(said) || (hides === "repo" && /^not_found: no such app or repository\b/i.test(said)) || (hides === "tool" && /^not_found: no such tool\b/i.test(said))), text: said }; | |
| 112 | 117 | } catch { |
| 113 | 118 | return { error: false, text }; |
| 114 | 119 | } |
| @@ -128,14 +133,14 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 128 | 133 | await refused("no repository of its own", "/api/v1/repos", token, { method: "POST", statuses: [403], error: ["unverified", "not_member"], json: { name: `${username}-repo` } }); |
| 129 | 134 | await refused("no app of its own", "/api/v1/apps", token, { method: "POST", statuses: [403], error: ["unverified", "not_member"], json: { name: `${username}-app` } }); |
| 130 | 135 | |
| 131 | const opened = await mcp("signup", { mode: "open", dryRun: false }); | |
| 136 | const opened = await mcp("signup", { mode: "open", dryRun: false }, token, "tool"); | |
| 132 | 137 | if (!opened.error) bad.push({ check: "no admin tool (MCP signup)", why: `signup answered a stranger: ${opened.text.slice(0, 80)}` }); |
| 133 | 138 | |
| 134 | 139 | if (options.ownerToken) { |
| 135 | 140 | const owned = options.ownerToken; |
| 136 | const listed = await call("/api/v1/repos?limit=100", owned); | |
| 137 | const items = Array.isArray(listed.body["items"]) ? (listed.body["items"] as Json[]) : []; | |
| 138 | if (listed.status !== 200) bad.push({ check: "owner token", why: `GET /api/v1/repos answered ${listed.status} for the owner's token` }); | |
| 141 | const ownedRepos = await call("/api/v1/repos?limit=100", owned); | |
| 142 | const items = Array.isArray(ownedRepos.body["items"]) ? (ownedRepos.body["items"] as Json[]) : []; | |
| 143 | if (ownedRepos.status !== 200) bad.push({ check: "owner token", why: `GET /api/v1/repos answered ${ownedRepos.status} for the owner's token` }); | |
| 139 | 144 | let aimed = 0; |
| 140 | 145 | for (const repo of items.slice(0, 50)) { |
| 141 | 146 | const owner = String(repo["owner"] ?? repo["ownerName"] ?? ""); |
| @@ -148,7 +153,7 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 148 | 153 | for (const sub of ["", "/issues", "/usage", "/runs", "/crew"]) |
| 149 | 154 | await refused(`no read of private ${label}`, at + sub, token, { statuses: [404] }); |
| 150 | 155 | for (const tool of ["repo", "usage"]) { |
| 151 | const got = await mcp(tool, { repo: label }); | |
| 156 | const got = await mcp(tool, { repo: label }, token, "repo"); | |
| 152 | 157 | if (!got.error) bad.push({ check: `no read of private ${label} (MCP ${tool})`, why: `${tool} answered a stranger: ${got.text.slice(0, 80)}` }); |
| 153 | 158 | } |
| 154 | 159 | const listing = await mcp("apps", {}); |
| @@ -158,7 +163,7 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 158 | 163 | // Steering: an assignment would start a crew run. Aim at a real issue |
| 159 | 164 | // and a real crew member, through the JSON door and the form door, and |
| 160 | 165 | // read the issue back as the owner: it must not have moved. |
| 161 | const mine = await mcp("make_it_yours", { repo: label, dryRun: false }); | |
| 166 | const mine = await mcp("make_it_yours", { repo: label, dryRun: false }, token, "repo"); | |
| 162 | 167 | if (!mine.error) bad.push({ check: `no copy of ${label} (MCP make_it_yours)`, why: `make_it_yours answered a stranger: ${mine.text.slice(0, 80)}` }); |
| 163 | 168 | const issues = await call(`${at}/issues?limit=1`, owned); |
| 164 | 169 | const issue = Array.isArray(issues.body["items"]) ? (issues.body["items"] as Json[])[0] : undefined; |
| @@ -203,7 +208,7 @@ export async function outsider(base: string, options: OutsiderOptions = {}): Pro | ||
| 203 | 208 | if (hidden && form.status !== 404 && form.status !== 403) |
| 204 | 209 | bad.push({ check: `no steering of ${label} (form)`, why: `a private repository's form answered ${form.status}, wanted 404` }); |
| 205 | 210 | // The MCP door: the tools that tell the crew or copy an app. |
| 206 | const told = await mcp("tell", { repo: label, text: "outsider suite: please change something" }); | |
| 211 | const told = await mcp("tell", { repo: label, text: "outsider suite: please change something" }, token, hidden ? "repo" : undefined); | |
| 207 | 212 | if (!told.error) bad.push({ check: `no steering of ${label} (MCP tell)`, why: "tell was accepted from a stranger" }); |
| 208 | 213 | const after = await call(`${at}/issues?limit=1`, owned); |
| 209 | 214 | const runsNow = await call(`${at}/runs?limit=100`, owned); |
test/outsider-suite.test.ts +32
| @@ -143,6 +143,38 @@ describe("outsider suite", () => { | ||
| 143 | 143 | expect(got.map((r) => r.check)).toContain("owner token"); |
| 144 | 144 | }); |
| 145 | 145 | |
| 146 | test("an MCP error that is not a refusal is not counted as one", async () => { | |
outdated
Passes on the base too: without a token the old code also returned only this refusal. It guards nothing the change added. Replace it with the case that moved: token given, nothing to aim at, so the no steering finding appears. |
||
| 147 | const said = (text: string) => | |
outdated
This test passes on the base, so it proves nothing about the change. Remove it along with the no-op guard move, or assert something that fails without the change. |
||
| 148 | (async (input: Request | string | URL, init?: RequestInit) => { | |
| 149 | if (new URL(input instanceof Request ? input.url : String(input)).pathname === "/mcp" && String(init?.body).includes('"name":"tell"') && String(init?.body).includes("rowan/lawn")) | |
| 150 | return Response.json({ jsonrpc: "2.0", id: 1, result: { isError: true, content: [{ type: "text", text }] } }); | |
| 151 | return fetch(input, init); | |
outdated
invalid_argument never matched the old regex, so this passes on main. Make the stub answer with a text the old regex accepted and the new one rejects (unauthorized, admin_only, or not_found on a non-hiding tell) so the test fails without the change. |
||
| 152 | }) as typeof fetch; | |
| 153 | for (const text of ["unauthorized: x", "not_found: nothing"]) { | |
| 154 | const got = await outsider(servers.url, { ownerToken, fetch: said(text), username: `stranger-err-${text.slice(0, 4)}` }); | |
| 155 | expect(got.map((r) => r.check)).toEqual(["no steering of rowan/lawn (MCP tell)"]); | |
| 156 | } | |
| 157 | }); | |
| 158 | ||
| 159 | test("a tool that has vanished is a moved door, not a refusal", async () => { | |
| 160 | const gone = (async (input: Request | string | URL, init?: RequestInit) => { | |
| 161 | if (new URL(input instanceof Request ? input.url : String(input)).pathname === "/mcp" && String(init?.body).includes('"name":"make_it_yours"')) | |
| 162 | return Response.json({ jsonrpc: "2.0", id: 1, result: { isError: true, content: [{ type: "text", text: "not_found: no such tool here" }] } }); | |
| 163 | return fetch(input, init); | |
| 164 | }) as typeof fetch; | |
| 165 | const got = await outsider(servers.url, { ownerToken, fetch: gone, username: "stranger-gone" }); | |
| 166 | expect(got.map((r) => r.check)).toEqual(["no copy of rowan/lawn (MCP make_it_yours)", "no copy of rowan/vault (MCP make_it_yours)"]); | |
| 167 | }); | |
| 168 | ||
| 169 | test("a hiding door that answers not_found still counts as refused", async () => { | |
| 170 | const hides = (async (input: Request | string | URL, init?: RequestInit) => { | |
| 171 | if (new URL(input instanceof Request ? input.url : String(input)).pathname === "/mcp" && String(init?.body).includes('"name":"repo"')) | |
| 172 | return Response.json({ jsonrpc: "2.0", id: 1, result: { isError: true, content: [{ type: "text", text: "not_found: no such app or repository (owner/name)" }] } }); | |
| 173 | return fetch(input, init); | |
| 174 | }) as typeof fetch; | |
| 175 | expect(await outsider(servers.url, { ownerToken, fetch: hides, username: "stranger-hides" })).toEqual([]); | |
| 176 | }); | |
| 177 | ||
| 146 | 178 | test("it says so when no account can be made", async () => { |
| 147 | 179 | setSignup(app.db, "closed", "test"); |
| 148 | 180 | const got = await outsider(servers.url, { username: "stranger3" }); |
new above
offline
Drop
toolfrom the hides alternation (keepno such app or repository); otherwise this line contradicts the comment on line 112 and a vanished tool passes as a refusal.