From 1d9226250195baae2269fadc82ad419076fc30a7 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 12:14:25 -0700 Subject: [PATCH 1/2] fix(api): block maintainer freeze override manifests --- src/api/routes.ts | 10 +++++++- test/unit/routes-focus-manifest.test.ts | 34 ++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/api/routes.ts b/src/api/routes.ts index 6869489a4f..d66ad8868d 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -2270,7 +2270,7 @@ export function createApp() { if (gate instanceof Response) return gate; const body = await c.req.json().catch(() => null); if (body === null) return c.json({ error: "invalid_json" }, 400); - const manifest = await upsertRepoFocusManifest(c.env, fullName, body, "api_record"); + const manifest = await upsertRepoFocusManifest(c.env, fullName, stripMaintainerFocusManifestSettings(body), "api_record"); return c.json({ repoFullName: fullName, manifest, policy: compileFocusManifestPolicy(manifest) }); }); @@ -5215,6 +5215,14 @@ const LINT_PR_TEXT_PATH = "/v1/lint/pr-text"; const VALIDATE_FOCUS_MANIFEST_PATH = "/v1/validate/focus-manifest"; const LINT_SLOP_RISK_PATH = "/v1/lint/slop-risk"; const LINT_ISSUE_SLOP_PATH = "/v1/lint/issue-slop"; +function stripMaintainerFocusManifestSettings(raw: unknown): unknown { + if (raw === null || typeof raw !== "object" || Array.isArray(raw)) return raw; + const record = raw as Record; + const settings = record.settings; + if (settings === null || typeof settings !== "object" || Array.isArray(settings) || !("agentGlobalFreezeOverride" in settings)) return raw; + const { agentGlobalFreezeOverride: _agentGlobalFreezeOverride, ...safeSettings } = settings; + return { ...record, settings: safeSettings }; +} // Contributor (miner) side of the extension (#556). Minted for NON-maintainer sign-ins; strictly // self-only — a token may only reach `/v1/extension/contributors//*`, enforced by the coarse // path check below plus `requireContributorAccess` (actor === login) in every handler. diff --git a/test/unit/routes-focus-manifest.test.ts b/test/unit/routes-focus-manifest.test.ts index 74aed49609..f16fced3a6 100644 --- a/test/unit/routes-focus-manifest.test.ts +++ b/test/unit/routes-focus-manifest.test.ts @@ -1,8 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; import { createSessionForGitHubUser } from "../../src/auth/security"; -import { upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { isDbFrozenForRepo, setGlobalAgentFrozen, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { getRepositoryCollaboratorPermission } from "../../src/github/app"; +import { resolveEffectiveSettings } from "../../src/signals/focus-manifest"; +import type { FocusManifest } from "../../src/signals/focus-manifest"; +import type { RepositorySettings } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; vi.mock("../../src/github/app", async (importOriginal) => ({ @@ -93,6 +96,35 @@ describe("focus-manifest route auth", () => { }); }); + it("strips operator-only freeze overrides from maintainer-writable focus-manifest updates", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 201, "repo-owner", "owned-repo"); + await setGlobalAgentFrozen(env, true, "operator"); + mockedPermission.mockResolvedValue("write"); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 201 }); + + const response = await app.request( + OWNED_REPO_PATH, + { + method: "PUT", + headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify({ wantedPaths: ["src/"], settings: { agentDryRun: true, agentGlobalFreezeOverride: true } }), + }, + env, + ); + + expect(response.status).toBe(200); + const body = await response.json() as { + manifest: { settings: { agentDryRun?: boolean; agentGlobalFreezeOverride?: boolean } }; + }; + expect(body.manifest.settings.agentDryRun).toBe(true); + expect(body.manifest.settings.agentGlobalFreezeOverride).toBeUndefined(); + const effective = resolveEffectiveSettings({ agentGlobalFreezeOverride: false } as RepositorySettings, body.manifest as FocusManifest); + expect(effective.agentGlobalFreezeOverride).toBe(false); + expect(await isDbFrozenForRepo(env, effective.agentGlobalFreezeOverride)).toBe(true); + }); + it("rejects focus-manifest writes from sessions without live GitHub write permission", async () => { const app = createApp(); const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); From 184580c65dae288a90131d0e102d613e8c3a6de7 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 00:42:59 -0700 Subject: [PATCH 2/2] test(api): cover every arm of stripMaintainerFocusManifestSettings's guards codecov/patch was failing at 85.71% -- only the "settings has the override key" happy path was tested. Adds cases for a non-object/array top-level body and a null/non-object/array/no-override-key settings value, so every OR-chain operand is independently exercised. Splits out the null-raw check (genuinely unreachable via the route's own prior 400-on-null-body guard) with a targeted v8-ignore rather than leaving it silently uncovered. --- src/api/routes.ts | 8 ++++++- test/unit/routes-focus-manifest.test.ts | 32 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/api/routes.ts b/src/api/routes.ts index 66e81e1d9e..7e24087632 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -5246,7 +5246,13 @@ const VALIDATE_FOCUS_MANIFEST_PATH = "/v1/validate/focus-manifest"; const LINT_SLOP_RISK_PATH = "/v1/lint/slop-risk"; const LINT_ISSUE_SLOP_PATH = "/v1/lint/issue-slop"; function stripMaintainerFocusManifestSettings(raw: unknown): unknown { - if (raw === null || typeof raw !== "object" || Array.isArray(raw)) return raw; + // Split out from the rest of the guard below: this call site's only caller already 400s on a null body + // before ever reaching here, so this specific arm is unreachable in practice -- kept as defense-in-depth + // (typeof null === "object" in JS, so without it a null raw would fall through to the property access + // below and throw) for any future caller of this currently-unexported function. + /* v8 ignore next */ + if (raw === null) return raw; + if (typeof raw !== "object" || Array.isArray(raw)) return raw; const record = raw as Record; const settings = record.settings; if (settings === null || typeof settings !== "object" || Array.isArray(settings) || !("agentGlobalFreezeOverride" in settings)) return raw; diff --git a/test/unit/routes-focus-manifest.test.ts b/test/unit/routes-focus-manifest.test.ts index f16fced3a6..8d1b4a619a 100644 --- a/test/unit/routes-focus-manifest.test.ts +++ b/test/unit/routes-focus-manifest.test.ts @@ -125,6 +125,38 @@ describe("focus-manifest route auth", () => { expect(await isDbFrozenForRepo(env, effective.agentGlobalFreezeOverride)).toBe(true); }); + // stripMaintainerFocusManifestSettings's guard is a compound OR chain (raw not-an-object / raw array / + // settings null / settings not-an-object / settings array / settings missing the override key) -- each of + // these leaves the body untouched (no strip), same as the pre-fix behavior, but for a different structural + // reason each time. Covering every arm here, not just the "settings HAS the key" happy path above. + it.each([ + ["a top-level non-object body", "just a string"], + ["a top-level array body", ["src/"]], + ["settings: null", { wantedPaths: ["src/"], settings: null }], + ["settings as a non-object", { wantedPaths: ["src/"], settings: "not-an-object" }], + ["settings as an array", { wantedPaths: ["src/"], settings: ["not", "a", "record"] }], + ["settings with no freeze-override key", { wantedPaths: ["src/"], settings: { agentDryRun: true } }], + ])("does not crash and passes the body through unstripped for %s", async (_label, body) => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 201, "repo-owner", "owned-repo"); + mockedPermission.mockResolvedValue("write"); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 201 }); + + const response = await app.request( + OWNED_REPO_PATH, + { + method: "PUT", + headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify(body), + }, + env, + ); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ repoFullName: "repo-owner/owned-repo" }); + }); + it("rejects focus-manifest writes from sessions without live GitHub write permission", async () => { const app = createApp(); const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" });