Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2272,7 +2272,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) });
});

Expand Down Expand Up @@ -5252,6 +5252,20 @@ 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 {
// 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<string, JsonValue>;
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/<self>/*`, enforced by the coarse
// path check below plus `requireContributorAccess` (actor === login) in every handler.
Expand Down
66 changes: 65 additions & 1 deletion test/unit/routes-focus-manifest.test.ts
Original file line number Diff line number Diff line change
@@ -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) => ({
Expand Down Expand Up @@ -93,6 +96,67 @@ 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);
});

// 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: "" });
Expand Down