From 7b1b99b3387a61ae6958ce0c92d88540e23fa769 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 17:06:50 -0700 Subject: [PATCH] fix(selfhost): require contents permission for auto-merge --- packages/gittensory-engine/src/index.ts | 1 + .../src/opportunity-freshness.ts | 4 +- src/api/routes.ts | 4 +- src/db/repositories.ts | 6 + src/github/app.ts | 32 ++++- src/github/backfill.ts | 124 ++++++++++++------ src/orb/app-auth.ts | 6 +- src/orb/broker-client.ts | 13 +- src/orb/broker.ts | 22 ++-- src/services/agent-action-executor.ts | 23 ++-- src/settings/agent-execution.ts | 84 +++++++++--- src/signals/settings-preview.ts | 17 ++- src/types.ts | 6 +- test/integration/orb-broker.test.ts | 47 +++++-- test/unit/agent-action-executor.test.ts | 45 ++++--- test/unit/agent-approval-queue.test.ts | 8 +- test/unit/agent-execution.test.ts | 39 ++++-- test/unit/backfill.test.ts | 115 +++++++++++++++- test/unit/github-app.test.ts | 65 +++++++++ test/unit/mcp-automation-state.test.ts | 14 +- .../unit/opportunity-branch-internals.test.ts | 4 +- test/unit/orb-app-auth.test.ts | 6 +- test/unit/orb-broker-client.test.ts | 13 +- test/unit/queue.test.ts | 14 +- test/unit/routes-agent-approval.test.ts | 2 +- test/unit/settings-preview.test.ts | 12 ++ 26 files changed, 556 insertions(+), 170 deletions(-) diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 4adbe14d99..43b5f0cf32 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -49,6 +49,7 @@ export { computeLaneFit, type GoalModelInput, } from "./goal-model.js"; +export { classifyContributorFit, type ContributorFit, type ContributorFitCheck, diff --git a/packages/gittensory-engine/src/opportunity-freshness.ts b/packages/gittensory-engine/src/opportunity-freshness.ts index 943ee78f49..fd2a09401a 100644 --- a/packages/gittensory-engine/src/opportunity-freshness.ts +++ b/packages/gittensory-engine/src/opportunity-freshness.ts @@ -27,9 +27,9 @@ function pickTimestamp(issue: FreshnessIssue): string | null { } function issueAgeDays(value: string | null, nowMs: number): number { - if (!value) return 0; + if (!value) return Number.POSITIVE_INFINITY; const parsed = Date.parse(value); - if (!Number.isFinite(parsed)) return 0; + if (!Number.isFinite(parsed)) return Number.POSITIVE_INFINITY; return Math.floor((nowMs - parsed) / 86_400_000); } diff --git a/src/api/routes.ts b/src/api/routes.ts index 71ab88acd8..f77494dcc0 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -2983,9 +2983,11 @@ export function createApp() { const auth = c.req.header("authorization") ?? ""; const secret = auth.startsWith("Bearer ") ? auth.slice(7).trim() : ""; if (!secret) return c.json({ error: "missing_enrollment_secret" }, 401); + const body = await c.req.json().catch(() => null); + const forceRefresh = typeof body === "object" && body !== null && (body as { forceRefresh?: unknown }).forceRefresh === true; let result: Awaited>; try { - result = await brokerOrbToken(c.env, secret); + result = await brokerOrbToken(c.env, secret, { forceRefresh }); } catch (error) { const message = error instanceof Error ? error.message : String(error); console.error(JSON.stringify({ level: "error", event: "orb_broker_mint_failed", message: message.slice(0, 200) })); diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 5d600ebbc4..b7edb6cfce 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -264,6 +264,12 @@ export async function getInstallation(env: Env, installationId: number): Promise return row ? toInstallationRecord(row) : null; } +export async function updateInstallationPermissions(env: Env, installationId: number, permissions: Record): Promise { + if (Object.keys(permissions).length === 0) return; + const db = getDb(env.DB); + await db.update(installations).set({ permissionsJson: jsonString(permissions), updatedAt: nowIso() }).where(eq(installations.id, installationId)); +} + export async function listInstallations(env: Env): Promise { const db = getDb(env.DB); const rows = await db.select().from(installations).orderBy(desc(installations.updatedAt)).limit(100); diff --git a/src/github/app.ts b/src/github/app.ts index a603e7f931..9d8b08a354 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -3,6 +3,7 @@ import { fetchBrokeredInstallationToken, isOrbBrokerMode, } from "../orb/broker-client"; +import { updateInstallationPermissions } from "../db/repositories"; import { clearGitHubResponseCacheForTest, githubRateLimitAdmissionKeyForInstallation, @@ -131,10 +132,12 @@ const inFlightMints = new Map>(); export async function createInstallationToken( env: Env, installationId: number, + options: { forceRefresh?: boolean } = {}, ): Promise { const cached = await readCachedToken(installationId); - if (cached && cached.expiresAtMs - TOKEN_SAFETY_MARGIN_MS > Date.now()) + if (!options.forceRefresh && cached && cached.expiresAtMs - TOKEN_SAFETY_MARGIN_MS > Date.now()) return cached.token; + if (options.forceRefresh) return mintInstallationToken(env, installationId, cached, true); const existing = inFlightMints.get(installationId); if (existing) return existing; // a concurrent caller is already minting for this install — join it const mint = mintInstallationToken(env, installationId, cached).finally(() => { @@ -157,6 +160,10 @@ export function isGitHubBadCredentialsError(error: unknown): boolean { return status === 401 || /bad credentials/i.test(errorMessage(error)); } +function isGitHubInstallationPermissionError(error: unknown): boolean { + return githubErrorStatus(error) === 403 && /resource not accessible by integration|not have permission/i.test(errorMessage(error)); +} + async function expireCachedInstallationToken( installationId: number, rejectedToken: string, @@ -175,7 +182,8 @@ export async function withInstallationTokenRetry( try { return await operation(token); } catch (error) { - if (!isGitHubBadCredentialsError(error)) throw error; + const refreshForPermission = isGitHubInstallationPermissionError(error); + if (!isGitHubBadCredentialsError(error) && !refreshForPermission) throw error; await expireCachedInstallationToken(installationId, token).catch( () => undefined, ); @@ -185,10 +193,11 @@ export async function withInstallationTokenRetry( event: "github_installation_token_rejected", installationId, status: githubErrorStatus(error), + reason: refreshForPermission ? "permission_scope" : "bad_credentials", message: errorMessage(error).slice(0, 200), }), ); - const freshToken = await createInstallationToken(env, installationId); + const freshToken = await createInstallationToken(env, installationId, { forceRefresh: refreshForPermission }); return await operation(freshToken); } } @@ -215,6 +224,7 @@ async function mintInstallationToken( env: Env, installationId: number, cached: { token: string; expiresAtMs: number } | null, + forceRefresh = false, ): Promise { // Self-host broker mode: a brokered self-host holds no App private key, so source the installation token from // the central Orb (enrollment secret → short-lived token) instead of minting locally. Cloud sets no enrollment @@ -222,11 +232,23 @@ async function mintInstallationToken( // self-host's single bound install). See src/orb/broker-client. if (isOrbBrokerMode(env)) { try { - const brokered = await fetchBrokeredInstallationToken(env); + const brokered = await fetchBrokeredInstallationToken(env, fetch, { forceRefresh }); await writeCachedToken(installationId, { token: brokered.token, expiresAtMs: brokered.expiresAtMs, }); + if (brokered.installationId === installationId && Object.keys(brokered.permissions).length > 0) { + await updateInstallationPermissions(env, installationId, brokered.permissions).catch((error) => { + console.warn( + JSON.stringify({ + level: "warn", + event: "github_installation_permissions_update_failed", + installationId, + message: errorMessage(error).slice(0, 200), + }), + ); + }); + } return brokered.token; } catch (error) { // Stale-token grace (#2): a brokered self-host holds no App key, so without this a single Orb mint failure @@ -234,7 +256,7 @@ async function mintInstallationToken( // token is STILL within its real expiry, serve it — a valid token beats a stalled review (NO dangerous reuse: // an actually-expired token is never served). Otherwise emit an alertable structured log and rethrow so the // queue's retry/DLQ handles a genuine outage. - if (cached && cached.expiresAtMs > Date.now()) { + if (!forceRefresh && cached && cached.expiresAtMs > Date.now()) { console.warn( JSON.stringify({ level: "warn", diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 99b96e7f1b..96a6448f61 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -38,10 +38,11 @@ import { upsertRepoSyncSegment, upsertRepoSyncState, upsertRepositoryFromGitHub, + updateInstallationPermissions, persistRepoSnapshot, extractLinkedIssueNumbers, } from "../db/repositories"; -import { agentRequiresPrWrite } from "../settings/agent-execution"; +import { agentRequiresContentsWrite, agentRequiresPrWrite } from "../settings/agent-execution"; import type { ContributorRepoStatRecord, GitHubRateLimitObservationRecord, @@ -828,12 +829,15 @@ export const OPTIONAL_CHECK_RUN_PERMISSION: Record = { export const OPTIONAL_PR_WRITE_PERMISSION: Record = { pull_requests: "write", }; +export const OPTIONAL_CONTENTS_WRITE_PERMISSION: Record = { + contents: "write", +}; export const REQUIRED_INSTALLATION_EVENTS = ["issues", "issue_comment", "pull_request", "repository"] as const; export const OPTIONAL_VISIBLE_INSTALLATION_EVENTS = ["installation_target", "installation_repositories"] as const; type InstallationModeImpact = { - mode: "comment" | "label" | "check_run" | "gate_check"; + mode: "comment" | "label" | "check_run" | "gate_check" | "agent_pr_action" | "agent_merge"; enabled: boolean; affectedRepoCount: number; requiredPermissions: Array<{ permission: string; requiredAccess: string; missing: boolean; optional: boolean }>; @@ -849,30 +853,30 @@ type InstallationEventDiagnostic = { action: string; }; -// Broker mode (#selfhost-runtime-drift): a brokered self-host holds no local GitHub App private key by design, -// and the token broker does not expose granted permissions/scopes today -- there is nothing to introspect. Report -// that plainly instead of running the local-mode remediation math against permissions/events that were never -// (and structurally cannot be) refreshed, which would otherwise read as either fabricated gaps or fabricated -// confirmations depending on whatever stale/default data happens to be stored. +// Broker mode (#selfhost-runtime-drift): a brokered self-host holds no local GitHub App private key by design. +// Permissions can be refreshed from the broker token response when available; event subscriptions still cannot be +// introspected through the broker, so the remediation text must keep that distinction explicit. function enrichBrokerInstallationHealth(health: InstallationHealthRecord) { const brokerHealthy = health.status === "healthy"; + const missingPermissions = new Set(health.missingPermissions); + const requiredPermissions = { + ...REQUIRED_INSTALLATION_PERMISSIONS, + ...(missingPermissions.has("checks") ? OPTIONAL_CHECK_RUN_PERMISSION : {}), + ...(missingPermissions.has("pull_requests") && permissionSatisfies(health.permissions.pull_requests, "read") ? OPTIONAL_PR_WRITE_PERMISSION : {}), + ...(missingPermissions.has("contents") ? OPTIONAL_CONTENTS_WRITE_PERMISSION : {}), + }; return { ...health, - requiredPermissions: REQUIRED_INSTALLATION_PERMISSIONS, + requiredPermissions, optionalPermissions: OPTIONAL_CHECK_RUN_PERMISSION, requiredEvents: [...REQUIRED_INSTALLATION_EVENTS], optionalVisibleEvents: [...OPTIONAL_VISIBLE_INSTALLATION_EVENTS], - // ok is always false here, regardless of brokerHealthy -- a reachable broker only proves it can mint - // tokens, never that this specific permission/event is actually granted (the broker exposes no - // introspection API for that today). Reporting ok: true for a check that was never verified would recreate - // the exact fabricated-success problem broker mode exists to avoid; brokerHealthy still drives repairSteps - // and the overall status above, just not these per-check flags. - permissionRemediation: Object.entries(REQUIRED_INSTALLATION_PERMISSIONS).map(([permission, access]) => ({ + permissionRemediation: Object.entries(requiredPermissions).map(([permission, access]) => ({ permission, requiredAccess: access, - currentAccess: "unavailable_in_broker_mode", - ok: false, - action: "Permission introspection is unavailable in broker mode.", + currentAccess: health.permissions[permission] ?? "unavailable_in_broker_mode", + ok: !missingPermissions.has(permission) && Boolean(health.permissions[permission]), + action: missingPermissions.has(permission) ? `Set repository permission ${permission} to ${access}.` : "No change needed.", })), eventRemediation: REQUIRED_INSTALLATION_EVENTS.map((event) => ({ event, @@ -883,7 +887,7 @@ function enrichBrokerInstallationHealth(health: InstallationHealthRecord) { ? [ "This is a brokered self-host (Orb token broker mode) -- it holds no local GitHub App private key by design.", "The token broker is reachable and minting installation tokens normally.", - "Permission/event introspection is not available through the token broker today.", + "Permission grants were refreshed from the broker token response when GitHub provided them; event-subscription introspection is unavailable in broker mode.", ] : [ "This is a brokered self-host (Orb token broker mode) -- it holds no local GitHub App private key by design.", @@ -909,6 +913,7 @@ export function enrichInstallationHealth(health: InstallationHealthRecord) { // Persisted health stores only the missing permission name. If pull_requests is already granted at read level, // a missing pull_requests entry can only mean an acting autonomy needs write; otherwise preserve baseline read. ...(missingPermissions.has("pull_requests") && permissionSatisfies(health.permissions.pull_requests, "read") ? OPTIONAL_PR_WRITE_PERMISSION : {}), + ...(missingPermissions.has("contents") ? OPTIONAL_CONTENTS_WRITE_PERMISSION : {}), }; return { ...health, @@ -949,14 +954,16 @@ export async function buildInstallationRepairDiagnostics(env: Env, health: Insta const labelRepoCount = installedSettings.filter(usesLabelMode).length; const checkRunRepoCount = installedSettings.filter((settings) => settings.checkRunMode === "enabled").length; const gateCheckRepoCount = installedSettings.filter((settings) => settings.gateCheckMode === "enabled").length; - const requiresPrWrite = installedSettings.some((settings) => agentRequiresPrWrite(settings.autonomy)); + const prWriteRepoCount = installedSettings.filter((settings) => agentRequiresPrWrite(settings.autonomy)).length; + const mergeRepoCount = installedSettings.filter((settings) => agentRequiresContentsWrite(settings.autonomy)).length; const missingPermissions = new Set(health.missingPermissions); const requiredEventSet = new Set(REQUIRED_INSTALLATION_EVENTS); const missingEvents = new Set(health.missingEvents.filter((event) => requiredEventSet.has(event))); const requiredPermissions = { ...REQUIRED_INSTALLATION_PERMISSIONS, ...(checkRunRepoCount > 0 || gateCheckRepoCount > 0 ? OPTIONAL_CHECK_RUN_PERMISSION : {}), - ...(requiresPrWrite ? OPTIONAL_PR_WRITE_PERMISSION : {}), // acting autonomy → pull_requests:write (#audit-install-health) + ...(prWriteRepoCount > 0 ? OPTIONAL_PR_WRITE_PERMISSION : {}), + ...(mergeRepoCount > 0 ? OPTIONAL_CONTENTS_WRITE_PERMISSION : {}), }; const optionalPermissions = checkRunRepoCount > 0 || gateCheckRepoCount > 0 ? {} : OPTIONAL_CHECK_RUN_PERMISSION; const modeImpacts: InstallationModeImpact[] = [ @@ -1004,6 +1011,32 @@ export async function buildInstallationRepairDiagnostics(env: Env, health: Insta ? "Review-agent check mode is enabled for at least one installed repo, so Checks: write is required." : "Checks: write is optional unless review-agent check mode is enabled for an installed repo.", }), + buildPermissionModeImpact({ + mode: "agent_pr_action", + enabled: prWriteRepoCount > 0, + affectedRepoCount: prWriteRepoCount, + permission: "pull_requests", + requiredAccess: "write", + missing: prWriteRepoCount > 0 && missingPermissions.has("pull_requests"), + optional: prWriteRepoCount === 0, + summary: + prWriteRepoCount > 0 + ? "Auto-maintain PR review, close, and update-branch actions are enabled for at least one installed repo, so Pull requests: write is required." + : "Pull requests: write is optional unless PR-state auto-maintain actions are enabled for an installed repo.", + }), + buildPermissionModeImpact({ + mode: "agent_merge", + enabled: mergeRepoCount > 0, + affectedRepoCount: mergeRepoCount, + permission: "contents", + requiredAccess: "write", + missing: mergeRepoCount > 0 && missingPermissions.has("contents"), + optional: mergeRepoCount === 0, + summary: + mergeRepoCount > 0 + ? "Auto-merge is enabled for at least one installed repo, so Contents: write is required by GitHub's merge endpoint." + : "Contents: write is optional unless auto-merge is enabled for an installed repo.", + }), ]; const eventDiagnostics: InstallationEventDiagnostic[] = [ ...REQUIRED_INSTALLATION_EVENTS.map((event) => ({ @@ -1114,12 +1147,22 @@ async function refreshInstallationHealthRecords(env: Env, installations: Install const { installation: currentInstallation, errorSummary, authMode } = await refreshStoredInstallation(env, installation); const installedRepos = repositories.filter((repo) => repo.installationId === currentInstallation.id && repo.isInstalled); const registeredInstalled = installedRepos.filter((repo) => repo.isRegistered); + const installedSettings = await Promise.all(installedRepos.map((repo) => getRepositorySettings(env, repo.fullName))); + const requiresChecks = installedSettings.some((settings) => settings.checkRunMode === "enabled"); + const requiresPrWrite = installedSettings.some((settings) => agentRequiresPrWrite(settings.autonomy)); + const requiresContentsWrite = installedSettings.some((settings) => agentRequiresContentsWrite(settings.autonomy)); + const requiredPermissions = { + ...REQUIRED_INSTALLATION_PERMISSIONS, + ...(requiresChecks ? OPTIONAL_CHECK_RUN_PERMISSION : {}), + // An acting autonomy upgrades the pull_requests requirement read -> write (spread last so it wins). (#audit-install-health) + ...(requiresPrWrite ? OPTIONAL_PR_WRITE_PERMISSION : {}), + ...(requiresContentsWrite ? OPTIONAL_CONTENTS_WRITE_PERMISSION : {}), + }; - // Broker mode (#selfhost-runtime-drift): the token broker exposes no permission/event introspection today, so - // there is nothing to compare against REQUIRED_INSTALLATION_PERMISSIONS -- computing "missing" from whatever - // (never-refreshed) permissions/events happen to be stored would fabricate a gap. Health instead reflects - // whether the broker itself is reachable and minting tokens; see enrichInstallationHealth's broker branch for - // the honest "introspection unavailable" remediation surfaced to callers. + // Broker mode (#selfhost-runtime-drift): the token broker can now expose the permission snapshot attached to + // the minted installation token, but it still cannot expose webhook event subscriptions. Recompute permission + // gaps when a broker snapshot is present; keep event gaps from the previous verified record instead of + // fabricating a clean event bill of health. // // Persistence (#selfhost-runtime-drift follow-up): writing missingPermissions/missingEvents as [] here reads, // to any OTHER consumer of InstallationHealthRecord that predates broker mode and doesn't branch on authMode @@ -1130,15 +1173,22 @@ async function refreshInstallationHealthRecords(env: Env, installations: Install // all has never been verified either way, so [] is the only honest starting point. if (authMode === "broker") { const previous = await getInstallationHealth(env, currentInstallation.id); + const hasBrokerPermissionSnapshot = Object.keys(currentInstallation.permissions).length > 0; + const missingPermissions = hasBrokerPermissionSnapshot + ? Object.entries(requiredPermissions) + .filter(([permission, expected]) => !permissionSatisfies(currentInstallation.permissions[permission], expected)) + .map(([permission]) => permission) + : (previous?.missingPermissions ?? ([] as string[])); + const missingEvents = previous?.missingEvents ?? ([] as string[]); const record = { installationId: currentInstallation.id, accountLogin: currentInstallation.accountLogin, repositorySelection: currentInstallation.repositorySelection, installedReposCount: installedRepos.length, registeredInstalledCount: registeredInstalled.length, - status: errorSummary ? ("needs_attention" as const) : ("healthy" as const), - missingPermissions: previous?.missingPermissions ?? ([] as string[]), - missingEvents: previous?.missingEvents ?? ([] as string[]), + status: errorSummary || missingPermissions.length > 0 || missingEvents.length > 0 ? ("needs_attention" as const) : ("healthy" as const), + missingPermissions, + missingEvents, permissions: currentInstallation.permissions, events: currentInstallation.events, checkedAt: nowIso(), @@ -1150,15 +1200,6 @@ async function refreshInstallationHealthRecords(env: Env, installations: Install continue; } - const installedSettings = await Promise.all(installedRepos.map((repo) => getRepositorySettings(env, repo.fullName))); - const requiresChecks = installedSettings.some((settings) => settings.checkRunMode === "enabled"); - const requiresPrWrite = installedSettings.some((settings) => agentRequiresPrWrite(settings.autonomy)); - const requiredPermissions = { - ...REQUIRED_INSTALLATION_PERMISSIONS, - ...(requiresChecks ? OPTIONAL_CHECK_RUN_PERMISSION : {}), - // An acting autonomy upgrades the pull_requests requirement read -> write (spread last so it wins). (#audit-install-health) - ...(requiresPrWrite ? OPTIONAL_PR_WRITE_PERMISSION : {}), - }; const missingPermissions = Object.entries(requiredPermissions) .filter(([permission, expected]) => !permissionSatisfies(currentInstallation.permissions[permission], expected)) .map(([permission]) => permission); @@ -1213,8 +1254,15 @@ async function refreshStoredInstallation( authMode: "broker", }; } + const refreshedInstallation = + minted.permissions && Object.keys(minted.permissions).length > 0 + ? { ...installation, permissions: minted.permissions, updatedAt: nowIso() } + : installation; + if (refreshedInstallation !== installation) { + await updateInstallationPermissions(env, installation.id, refreshedInstallation.permissions); + } incr("gittensory_installation_health_broker_probe_total", { result: "ok" }); - return { installation, authMode: "broker" }; + return { installation: refreshedInstallation, authMode: "broker" }; } catch (error) { incr("gittensory_installation_health_broker_probe_total", { result: "failed" }); return { diff --git a/src/orb/app-auth.ts b/src/orb/app-auth.ts index 9d2b2af32b..8608e83658 100644 --- a/src/orb/app-auth.ts +++ b/src/orb/app-auth.ts @@ -61,7 +61,7 @@ const ORB_TOKEN_MINT_TIMEOUT_MS = 25_000; /** Mints a short-lived GitHub installation access token for one installation — the broker primitive the * self-hosted container ultimately receives (after enrollment). brokerOrbToken caches the result; this always mints. */ -export async function createOrbInstallationToken(env: Env, installationId: number): Promise<{ token: string; expiresAt: string }> { +export async function createOrbInstallationToken(env: Env, installationId: number): Promise<{ token: string; expiresAt: string; permissions: Record }> { const jwt = await createOrbAppJwt(env); const response = await timeoutFetch(`https://api.github.com/app/installations/${installationId}/access_tokens`, { method: "POST", @@ -72,8 +72,8 @@ export async function createOrbInstallationToken(env: Env, installationId: numbe const body = await response.text(); throw new Error(`Failed to create Orb installation token (${response.status}): ${body.slice(0, 200)}`); } - const payload = (await response.json()) as { token?: string; expires_at?: string }; + const payload = (await response.json()) as { token?: string; expires_at?: string; permissions?: Record }; if (!payload.token) throw new Error("Orb installation token response did not include a token."); // Surface GitHub's real expiry (~1h) so the broker never invents one; absent only on a malformed response. - return { token: payload.token, expiresAt: payload.expires_at ?? "" }; + return { token: payload.token, expiresAt: payload.expires_at ?? "", permissions: payload.permissions ?? {} }; } diff --git a/src/orb/broker-client.ts b/src/orb/broker-client.ts index 288f042079..b26fd43b4c 100644 --- a/src/orb/broker-client.ts +++ b/src/orb/broker-client.ts @@ -44,7 +44,7 @@ export function isOrbBrokerMode(env: { ORB_ENROLLMENT_SECRET?: string | undefine return Boolean(env.ORB_ENROLLMENT_SECRET); } -export type BrokeredInstallationToken = { token: string; installationId: number; expiresAtMs: number }; +export type BrokeredInstallationToken = { token: string; installationId: number; expiresAtMs: number; permissions: Record }; /** Exchange the enrollment secret for a brokered installation token + its expiry (ms epoch). Throws on a non-OK * response (401 invalid_enrollment / 403 installation_not_eligible / 5xx) or a tokenless body — a brokered @@ -53,17 +53,22 @@ export type BrokeredInstallationToken = { token: string; installationId: number; export async function fetchBrokeredInstallationToken( env: { ORB_ENROLLMENT_SECRET?: string | undefined; ORB_BROKER_URL?: string | undefined }, fetchImpl: typeof fetch = fetch, + options: { forceRefresh?: boolean } = {}, ): Promise { const base = orbBrokerBaseUrl(env); const response = await fetchImpl(`${base}/v1/orb/token`, { method: "POST", - headers: { authorization: `Bearer ${env.ORB_ENROLLMENT_SECRET ?? ""}` }, + headers: { + authorization: `Bearer ${env.ORB_ENROLLMENT_SECRET ?? ""}`, + ...(options.forceRefresh ? { "content-type": "application/json" } : {}), + }, + ...(options.forceRefresh ? { body: JSON.stringify({ forceRefresh: true }) } : {}), signal: AbortSignal.timeout(BROKER_TIMEOUT_MS), }); if (!response.ok) { throw new Error(`Orb broker token exchange failed (${response.status}).`); } - const payload = (await response.json()) as { token?: string; installationId?: number; expiresAt?: string }; + const payload = (await response.json()) as { token?: string; installationId?: number; expiresAt?: string; permissions?: Record }; if (!payload.token) { throw new Error("Orb broker token response did not include a token."); } @@ -72,7 +77,7 @@ export async function fetchBrokeredInstallationToken( // false for NaN — re-minting a brokered token on every GitHub call instead of caching it for ~an hour. const parsedExpiry = payload.expiresAt ? Date.parse(payload.expiresAt) : Number.NaN; const expiresAtMs = Number.isFinite(parsedExpiry) ? parsedExpiry : Date.now() + 50 * 60_000; - return { token: payload.token, installationId: payload.installationId ?? 0, expiresAtMs }; + return { token: payload.token, installationId: payload.installationId ?? 0, expiresAtMs, permissions: payload.permissions ?? {} }; } // Diagnosing a broker register failure (#selfhost-runtime-drift) needs more than a bare status code, but the diff --git a/src/orb/broker.ts b/src/orb/broker.ts index d2acae01ea..5b0fb1d8f7 100644 --- a/src/orb/broker.ts +++ b/src/orb/broker.ts @@ -48,12 +48,14 @@ export async function issueOrbEnrollment( return { enrollId, secret }; } -export type BrokerResult = { token: string; installationId: number; expiresAt: string } | { error: "invalid_enrollment" | "installation_not_eligible" | "broker_misconfigured" }; +export type BrokerResult = + | { token: string; installationId: number; expiresAt: string; permissions: Record } + | { error: "invalid_enrollment" | "installation_not_eligible" | "broker_misconfigured" }; /** The container's token-exchange: a valid enrollment secret → a short-lived installation token for the BOUND * install. installation_id is read from the enrollment row, never the caller; the install must still be * registered=1 and neither suspended nor removed at mint time (the gate is re-checked, not trusted from issue). */ -export async function brokerOrbToken(env: Env, secret: string): Promise { +export async function brokerOrbToken(env: Env, secret: string, options: { forceRefresh?: boolean } = {}): Promise { // Warn when TOKEN_ENCRYPTION_SECRET is absent — without it, the broker cache is bypassed and every exchange hits // GitHub's token endpoint, dramatically increasing exposure to throttle-induced failures. if (!env.TOKEN_ENCRYPTION_SECRET) { @@ -73,10 +75,10 @@ export async function brokerOrbToken(env: Env, secret: string): Promise engine timeouts -> unavailable orb). // The token is cached encrypted-at-rest (AES-256-GCM via TOKEN_ENCRYPTION_SECRET); with no key set the cache is // skipped and we mint every call exactly as before. - const cached = await readCachedOrbToken(env, row.cached_token_json); + const cached = options.forceRefresh ? null : await readCachedOrbToken(env, row.cached_token_json); if (cached) { await touchLastToken(env, row.enroll_id); - return { token: cached.token, installationId: row.installation_id, expiresAt: cached.expiresAt }; + return { token: cached.token, installationId: row.installation_id, expiresAt: cached.expiresAt, permissions: cached.permissions }; } // Validate Orb App credentials only now that the caller is proven to hold a valid, eligible enrollment — NOT // up front. Checking credentials before the enrollment lookup would let an unauthenticated caller (any bad @@ -89,7 +91,7 @@ export async function brokerOrbToken(env: Env, secret: string): Promise { @@ -102,13 +104,13 @@ async function touchLastToken(env: Env, enrollId: string): Promise { /** Decrypt + return the cached installation token when present and still safely before expiry; null (→ re-mint) on * no key, no cache, an expired/unparseable entry, or any decrypt failure (e.g. a rotated encryption key). */ -async function readCachedOrbToken(env: Env, cachedJson: string | null): Promise<{ token: string; expiresAt: string } | null> { +async function readCachedOrbToken(env: Env, cachedJson: string | null): Promise<{ token: string; expiresAt: string; permissions: Record } | null> { if (!env.TOKEN_ENCRYPTION_SECRET || !cachedJson) return null; try { - const entry = JSON.parse(cachedJson) as { ciphertext: string; iv: string; salt: string | null; expiresAt: string }; + const entry = JSON.parse(cachedJson) as { ciphertext: string; iv: string; salt: string | null; expiresAt: string; permissions?: Record }; if (!(Date.parse(entry.expiresAt) - Date.now() >= ORB_TOKEN_CACHE_MIN_REMAINING_MS)) return null; const token = await decryptSecret(entry.ciphertext, entry.iv, env.TOKEN_ENCRYPTION_SECRET, entry.salt); - return { token, expiresAt: entry.expiresAt }; + return { token, expiresAt: entry.expiresAt, permissions: entry.permissions ?? {} }; } catch { return null; } @@ -116,11 +118,11 @@ async function readCachedOrbToken(env: Env, cachedJson: string | null): Promise< /** Cache the freshly minted token (encrypted) on the enrollment row. Best-effort + fail-safe: a cache-write error * must never fail a valid token exchange — the next call simply re-mints. No-op without an encryption key. */ -async function cacheOrbToken(env: Env, enrollId: string, minted: { token: string; expiresAt: string }): Promise { +async function cacheOrbToken(env: Env, enrollId: string, minted: { token: string; expiresAt: string; permissions: Record }): Promise { if (!env.TOKEN_ENCRYPTION_SECRET) return; try { const enc = await encryptSecret(minted.token, env.TOKEN_ENCRYPTION_SECRET); - const json = JSON.stringify({ ciphertext: enc.ciphertext, iv: enc.iv, salt: enc.salt, expiresAt: minted.expiresAt }); + const json = JSON.stringify({ ciphertext: enc.ciphertext, iv: enc.iv, salt: enc.salt, expiresAt: minted.expiresAt, permissions: minted.permissions }); await env.DB.prepare("UPDATE orb_enrollments SET cached_token_json = ? WHERE enroll_id = ?").bind(json, enrollId).run(); } catch (error) { console.warn(JSON.stringify({ level: "warn", event: "orb_token_cache_write_failed", enrollId, message: String(error).slice(0, 120) })); diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index b6efd3c2ee..2a0d7f57ce 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -22,7 +22,7 @@ import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels import { closeIssue, closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../github/pr-actions"; import { fetchPullRequestFreshness, pullRequestFreshnessDetail } from "../github/pr-freshness"; import { isActingAutonomyLevel, resolveAutonomy } from "../settings/autonomy"; -import { buildAgentActionAudit, isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness, type AgentActionMode } from "../settings/agent-execution"; +import { buildAgentActionAudit, formatAgentPermissionDenial, isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness, type AgentActionMode } from "../settings/agent-execution"; import type { PlannedAgentAction } from "../settings/agent-actions"; import type { AgentActionClass, AgentPendingActionParams, AutonomyLevel, AutonomyPolicy } from "../types"; import { errorMessage } from "../utils/json"; @@ -40,10 +40,9 @@ import { incr } from "../selfhost/metrics"; // autonomy (the config IS the authorization; there is no human commenter to authorize, unlike #824). const AGENT_ACTOR = "gittensory"; -// The PR-state action classes that require GitHub `pull_requests: write`. `label` mutates via the Issues API -// (`issues: write`, always held), so it is exempt from the write-permission readiness gate. Exported so the -// agent-execution test can enforce the invariant that every member is also counted by agentRequiresPrWrite -// (PR_WRITE_ACTION_CLASSES is a superset), so this runtime guard never disagrees with the readiness gate. +// The PR-visible action classes that require an elevated GitHub App write permission. Most use +// `pull_requests: write`; merge uses `contents: write`; `label` mutates through the Issues API, so it is exempt +// from this readiness gate. export const PR_WRITE_CLASSES = new Set(["request_changes", "approve", "merge", "close", "update_branch"]); const INSTALLATION_HEALTH_REFRESH_COOLDOWN_MS = 5 * 60 * 1000; @@ -212,11 +211,11 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE await audit("queued", `awaiting maintainer approval — ${action.reason}`); continue; } - // 5) Write-permission readiness: a PR-write action needs `pull_requests: write` granted. Checked here -- - // before the freshness/live-CI GitHub calls below -- so a known-denied action never spends that API - // budget on an outcome that cannot change until the maintainer re-consents (#selfhost-runtime-drift). - // `label` mutates via the Issues API (`issues: write`, always held), so it is exempt from this gate. - if (PR_WRITE_CLASSES.has(action.actionClass) && resolveAgentPermissionReadiness({ autonomy: ctx.autonomy, installationPermissions: ctx.installationPermissions }) !== "ready") { + // 5) Write-permission readiness: a PR-visible action needs its exact GitHub App write permission granted. + // Merge is Contents: write, while review/close/update_branch are Pull requests: write. Checked here + // before the freshness/live-CI GitHub calls below so a known-denied action never spends that API budget on + // an outcome that cannot change until the maintainer re-consents (#selfhost-runtime-drift). + if (PR_WRITE_CLASSES.has(action.actionClass) && resolveAgentPermissionReadiness({ autonomy: ctx.autonomy, installationPermissions: ctx.installationPermissions, actionClass: action.actionClass }) !== "ready") { incr("gittensory_agent_action_permission_denied_total", { actionClass: action.actionClass }); const cooldownKey = writePermissionDenialKey(ctx.installationId, ctx.repoFullName, ctx.pullNumber, action.actionClass); if (shouldSuppressWritePermissionDenial(cooldownKey, Date.now())) { @@ -226,11 +225,11 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE outcomes.push({ actionClass: action.actionClass, outcome: "denied", - detail: "pull_requests: write not granted — maintainer must re-consent (suppressed repeat)", + detail: formatAgentPermissionDenial({ autonomy: ctx.autonomy, installationPermissions: ctx.installationPermissions, actionClass: action.actionClass, suppressed: true }), }); continue; } - await audit("denied", "pull_requests: write not granted — maintainer must re-consent"); + await audit("denied", formatAgentPermissionDenial({ autonomy: ctx.autonomy, installationPermissions: ctx.installationPermissions, actionClass: action.actionClass })); markWritePermissionDenialAudited(cooldownKey, Date.now()); continue; } diff --git a/src/settings/agent-execution.ts b/src/settings/agent-execution.ts index 5618a9fa8a..8eb2875b2c 100644 --- a/src/settings/agent-execution.ts +++ b/src/settings/agent-execution.ts @@ -1,16 +1,17 @@ import type { AgentActionClass, AuditEventRecord, AutonomyLevel, AutonomyPolicy } from "../types"; import { isActingAutonomyLevel, resolveAutonomy } from "./autonomy"; -// The action classes that mutate a PR's review / merge / close / head state — these need GitHub -// `pull_requests: write`. (`label` mutates via the Issues API, which the App already holds `issues: write` for.) -// INVARIANT: the executor's PR_WRITE_CLASSES (src/services/agent-action-executor.ts) must be a SUBSET of this list -// — every class the runtime readiness guard treats as a PR-write must be counted by agentRequiresPrWrite, or the -// readiness gate under-reports and disagrees with the executor. (This list may be a superset: it also carries the -// advisory `review` class for conservatism.) `update_branch` (PUT /pulls/{n}/update-branch) is a PR-write the -// executor gates; omitting it here graded an update_branch-only autonomy "not_required", so the executor's -// readiness guard denied it even WITH pull_requests:write granted (and it would 403 if it slipped). The -// agent-execution test enforces this subset invariant against the exported PR_WRITE_CLASSES. (#audit-update-branch) -const PR_WRITE_ACTION_CLASSES: readonly AgentActionClass[] = ["review", "request_changes", "approve", "merge", "close", "update_branch"]; +// The action classes that mutate a PR's review / close / head state through Pull Request endpoints. Merge is +// deliberately separate: GitHub's merge endpoint requires `contents: write`, so treating it as only +// `pull_requests: write` lets a repo look ready while the live merge 403s. +// `update_branch` (PUT /pulls/{n}/update-branch) is a PR-write the executor gates; omitting it here graded an +// update_branch-only autonomy "not_required", so the executor's readiness guard denied it even WITH +// pull_requests:write granted (and it would 403 if it slipped). Tests keep the action-specific requirements in +// sync with the executor's exported write-action set. (#audit-update-branch) +const PR_WRITE_ACTION_CLASSES: readonly AgentActionClass[] = ["review", "request_changes", "approve", "close", "update_branch"]; +const CONTENTS_WRITE_ACTION_CLASSES: readonly AgentActionClass[] = ["merge"]; + +export type AgentPermissionRequirement = { permission: string; requiredAccess: "write" }; // Whether the agent actually executes an action, only logs what it WOULD do, or is halted entirely (#776). export type AgentActionMode = "paused" | "dry_run" | "live"; @@ -78,15 +79,68 @@ export function agentRequiresPrWrite(autonomy: AutonomyPolicy | null | undefined return PR_WRITE_ACTION_CLASSES.some((actionClass) => isActingAutonomyLevel(resolveAutonomy(autonomy, actionClass))); } +/** True when the configured autonomy can execute a merge, which GitHub authorizes via Contents: write. */ +export function agentRequiresContentsWrite(autonomy: AutonomyPolicy | null | undefined): boolean { + return CONTENTS_WRITE_ACTION_CLASSES.some((actionClass) => isActingAutonomyLevel(resolveAutonomy(autonomy, actionClass))); +} + export type AgentPermissionReadiness = "not_required" | "ready" | "reconsent_required"; +function addRequirementOnce(requirements: AgentPermissionRequirement[], requirement: AgentPermissionRequirement): void { + if (requirements.some((entry) => entry.permission === requirement.permission)) return; + requirements.push(requirement); +} + +export function requiredAgentActionPermissions( + autonomy: AutonomyPolicy | null | undefined, + actionClass?: AgentActionClass | null | undefined, +): AgentPermissionRequirement[] { + const candidates = actionClass ? [actionClass] : [...PR_WRITE_ACTION_CLASSES, ...CONTENTS_WRITE_ACTION_CLASSES]; + const requirements: AgentPermissionRequirement[] = []; + for (const candidate of candidates) { + if (!isActingAutonomyLevel(resolveAutonomy(autonomy, candidate))) continue; + if (PR_WRITE_ACTION_CLASSES.includes(candidate)) addRequirementOnce(requirements, { permission: "pull_requests", requiredAccess: "write" }); + if (CONTENTS_WRITE_ACTION_CLASSES.includes(candidate)) addRequirementOnce(requirements, { permission: "contents", requiredAccess: "write" }); + } + return requirements; +} + +export function missingAgentActionPermissions(input: { + autonomy: AutonomyPolicy | null | undefined; + installationPermissions: Record | null | undefined; + actionClass?: AgentActionClass | null | undefined; +}): AgentPermissionRequirement[] { + return requiredAgentActionPermissions(input.autonomy, input.actionClass).filter( + (requirement) => input.installationPermissions?.[requirement.permission] !== requirement.requiredAccess, + ); +} + +export function formatAgentPermissionDenial(input: { + autonomy: AutonomyPolicy | null | undefined; + installationPermissions: Record | null | undefined; + actionClass?: AgentActionClass | null | undefined; + suppressed?: boolean | undefined; +}): string { + const missing = missingAgentActionPermissions(input); + const summary = + missing.length > 0 + ? missing.map((requirement) => `${requirement.permission}: ${requirement.requiredAccess}`).join(", ") + : "required GitHub App permission"; + return `${summary} not granted — maintainer must re-consent${input.suppressed ? " (suppressed repeat)" : ""}`; +} + /** * Whether the installation grants the write scope the configured auto-maintain actions need (#775). The action - * layer (#778) consults this before executing a PR-write action: `not_required` = no acting PR-write level is - * configured; `ready` = the App holds `pull_requests: write`; `reconsent_required` = the maintainer must + * layer (#778) consults this before executing a GitHub mutation: `not_required` = no acting level needs a write + * permission; `ready` = the App holds every required write permission; `reconsent_required` = the maintainer must * re-authorize the App with the upgraded permission. Pure. */ -export function resolveAgentPermissionReadiness(input: { autonomy: AutonomyPolicy | null | undefined; installationPermissions: Record | null | undefined }): AgentPermissionReadiness { - if (!agentRequiresPrWrite(input.autonomy)) return "not_required"; - return input.installationPermissions?.pull_requests === "write" ? "ready" : "reconsent_required"; +export function resolveAgentPermissionReadiness(input: { + autonomy: AutonomyPolicy | null | undefined; + installationPermissions: Record | null | undefined; + actionClass?: AgentActionClass | null | undefined; +}): AgentPermissionReadiness { + const required = requiredAgentActionPermissions(input.autonomy, input.actionClass); + if (required.length === 0) return "not_required"; + return missingAgentActionPermissions(input).length === 0 ? "ready" : "reconsent_required"; } diff --git a/src/signals/settings-preview.ts b/src/signals/settings-preview.ts index 265dc1d0a5..d43c7718e2 100644 --- a/src/signals/settings-preview.ts +++ b/src/signals/settings-preview.ts @@ -15,6 +15,7 @@ import { } from "./engine"; import { REQUIRED_INSTALLATION_PERMISSIONS } from "../github/backfill"; import { GITTENSORY_GATE_CHECK_NAME } from "../review/check-names"; +import { requiredAgentActionPermissions } from "../settings/agent-execution"; export function hasVisiblePrSurface(settings: RepositorySettings): boolean { return settings.publicSurface !== "off" || settings.checkRunMode === "enabled" || settings.gateCheckMode === "enabled"; @@ -522,18 +523,24 @@ function requiredInstallPermissions(settings: RepositorySettings, decision: Publ ); if (writesPrPublicSurface(settings, decision)) permissions.add("issues: write"); if (decision.willCheckRun || settings.checkRunMode === "enabled" || settings.gateCheckMode === "enabled") permissions.add("checks: write"); + for (const requirement of requiredAgentActionPermissions(settings.autonomy)) { + permissions.add(`${requirement.permission}: ${requirement.requiredAccess}`); + } return [...permissions]; } function activeMissingPermissions(settings: RepositorySettings, decision: PublicSurfaceDecision, installation: InstallationHealthSummary | null): string[] { if (!installation) return []; const missing = new Set(installation.missingPermissions); - const active: string[] = []; - if (missing.has("pull_requests")) active.push("pull_requests"); + const active = new Set(); + if (missing.has("pull_requests")) active.add("pull_requests"); + for (const requirement of requiredAgentActionPermissions(settings.autonomy)) { + if (missing.has(requirement.permission)) active.add(requirement.permission); + } // Comment/label output is gated on issues:write (Issues endpoints), not pull_requests:write. - if (writesPrPublicSurface(settings, decision) && missing.has("issues")) active.push("issues"); - if ((decision.willCheckRun || settings.checkRunMode === "enabled" || settings.gateCheckMode === "enabled") && missing.has("checks")) active.push("checks"); - return active; + if (writesPrPublicSurface(settings, decision) && missing.has("issues")) active.add("issues"); + if ((decision.willCheckRun || settings.checkRunMode === "enabled" || settings.gateCheckMode === "enabled") && missing.has("checks")) active.add("checks"); + return [...active]; } function permissionSummary(installation: InstallationHealthSummary | null, missing: string[], missingEvents: string[]): string { diff --git a/src/types.ts b/src/types.ts index 368e7514cd..78be7887fb 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1478,9 +1478,9 @@ export type InstallationHealthRecord = { checkedAt: string; errorSummary?: string | null | undefined; // "broker" = a brokered self-host (ORB_ENROLLMENT_SECRET set, no local GitHub App private key by design). - // Permission/event introspection is not available through the token broker today, so missingPermissions / - // missingEvents are always [] in broker mode -- an EMPTY array there means "unchecked", not "all satisfied" - // the way it does for "local" mode. Consumers must branch on authMode, not infer it from empty arrays alone. + // Permission snapshots are available only after the broker returns token permissions; event subscriptions are + // not introspectable through the broker. Consumers must branch on authMode, not infer certainty from empty + // arrays alone. authMode: "local" | "broker"; }; diff --git a/test/integration/orb-broker.test.ts b/test/integration/orb-broker.test.ts index fe1b58f901..3aec4fc20c 100644 --- a/test/integration/orb-broker.test.ts +++ b/test/integration/orb-broker.test.ts @@ -24,12 +24,13 @@ const brokerEnvMissingAppCreds = async (missing: "id" | "key" | "both", over: Pa if (missing !== "key") base.ORB_GITHUB_APP_PRIVATE_KEY = await pkcs8Pem(); return createTestEnv(base); }; -const tokenFetch = (token = "ghs_broker", expires = "2026-06-25T08:00:00Z") => vi.stubGlobal("fetch", async () => Response.json({ token, expires_at: expires })); -const countingTokenFetch = (expires = "2026-06-25T08:00:00Z") => { +const tokenFetch = (token = "ghs_broker", expires = "2026-06-25T08:00:00Z", permissions: Record = {}) => + vi.stubGlobal("fetch", async () => Response.json({ token, expires_at: expires, permissions })); +const countingTokenFetch = (expires = "2026-06-25T08:00:00Z", permissions: Record = {}) => { let calls = 0; vi.stubGlobal("fetch", async () => { calls += 1; - return Response.json({ token: `ghs_minted_${calls}`, expires_at: expires }); + return Response.json({ token: `ghs_minted_${calls}`, expires_at: expires, permissions }); }); return () => calls; }; @@ -66,8 +67,8 @@ describe("brokerOrbToken", () => { const e = await brokerEnv(); await seedInstall(e, 300, { registered: 1 }); const { secret } = (await issueOrbEnrollment(e, 300)) as { secret: string }; - tokenFetch("ghs_minted", "2026-06-25T08:00:00Z"); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted", installationId: 300, expiresAt: "2026-06-25T08:00:00Z" }); + tokenFetch("ghs_minted", "2026-06-25T08:00:00Z", { contents: "write" }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted", installationId: 300, expiresAt: "2026-06-25T08:00:00Z", permissions: { contents: "write" } }); expect((await db(e).prepare("SELECT last_token_at FROM orb_enrollments WHERE installation_id=300").first<{ last_token_at: string | null }>())?.last_token_at).not.toBeNull(); }); @@ -77,16 +78,38 @@ describe("brokerOrbToken", () => { const e = await brokerEnv({ TOKEN_ENCRYPTION_SECRET: "orb-cache-test-secret" }); await seedInstall(e, 303, { registered: 1 }); const { secret } = (await issueOrbEnrollment(e, 303)) as { secret: string }; - const fetchCalls = countingTokenFetch("2026-06-25T08:00:00Z"); + const fetchCalls = countingTokenFetch("2026-06-25T08:00:00Z", { contents: "write", pull_requests: "write" }); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 303, expiresAt: "2026-06-25T08:00:00Z" }); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 303, expiresAt: "2026-06-25T08:00:00Z" }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 303, expiresAt: "2026-06-25T08:00:00Z", permissions: { contents: "write", pull_requests: "write" } }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 303, expiresAt: "2026-06-25T08:00:00Z", permissions: { contents: "write", pull_requests: "write" } }); expect(fetchCalls()).toBe(1); const row = await db(e).prepare("SELECT cached_token_json FROM orb_enrollments WHERE installation_id=303").first<{ cached_token_json: string }>(); expect(row?.cached_token_json).toContain("ciphertext"); expect(row?.cached_token_json).not.toContain("ghs_minted_1"); }); + it("forceRefresh bypasses a still-fresh broker cache and replaces the returned permission snapshot", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-06-25T07:00:00Z")); + const e = await brokerEnv({ TOKEN_ENCRYPTION_SECRET: "orb-cache-test-secret" }); + await seedInstall(e, 307, { registered: 1 }); + const { secret } = (await issueOrbEnrollment(e, 307)) as { secret: string }; + let calls = 0; + vi.stubGlobal("fetch", async () => { + calls += 1; + return Response.json({ + token: `ghs_minted_${calls}`, + expires_at: "2026-06-25T08:00:00Z", + permissions: calls === 1 ? { contents: "read" } : { contents: "write" }, + }); + }); + + expect(await brokerOrbToken(e, secret)).toMatchObject({ token: "ghs_minted_1", permissions: { contents: "read" } }); + expect(await brokerOrbToken(e, secret, { forceRefresh: true })).toMatchObject({ token: "ghs_minted_2", permissions: { contents: "write" } }); + expect(await brokerOrbToken(e, secret)).toMatchObject({ token: "ghs_minted_2", permissions: { contents: "write" } }); + expect(calls).toBe(2); + }); + it("remints when the encrypted cache is absent, expired, or unreadable", async () => { vi.useFakeTimers(); vi.setSystemTime(new Date("2026-06-25T07:00:00Z")); @@ -119,7 +142,7 @@ describe("brokerOrbToken", () => { return originalPrepare(sql); }); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_cache_write_failed", installationId: 305, expiresAt: "2026-06-25T08:00:00Z" }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_cache_write_failed", installationId: 305, expiresAt: "2026-06-25T08:00:00Z", permissions: {} }); expect(warn).toHaveBeenCalledWith(expect.stringContaining("orb_token_cache_write_failed")); }); @@ -141,7 +164,7 @@ describe("brokerOrbToken", () => { return originalPrepare(sql); }); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 306, expiresAt: "2026-06-25T08:00:00Z" }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted_1", installationId: 306, expiresAt: "2026-06-25T08:00:00Z", permissions: {} }); expect(fetchCalls()).toBe(1); expect(warn).toHaveBeenCalledWith(expect.stringContaining("orb_token_last_touch_failed")); }); @@ -198,11 +221,11 @@ describe("brokerOrbToken", () => { await seedInstall(e, 313, { registered: 1 }); const { secret } = (await issueOrbEnrollment(e, 313)) as { secret: string }; tokenFetch("ghs_minted", "2026-06-25T08:00:00Z"); - expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z" }); + expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z", permissions: {} }); // Drop the App credentials AFTER the initial mint+cache — a still-fresh cache hit must not need them. const eNoCreds = await brokerEnvMissingAppCreds("both", { TOKEN_ENCRYPTION_SECRET: "orb-cache-test-secret", DB: e.DB }); - expect(await brokerOrbToken(eNoCreds, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z" }); + expect(await brokerOrbToken(eNoCreds, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z", permissions: {} }); }); }); diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index cd47fba9e3..cf5f335751 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -69,7 +69,7 @@ function ctx(over: Partial = {}): AgentActionExecut autonomy: { label: "auto", request_changes: "auto", approve: "auto", merge: "auto", close: "auto", update_branch: "auto" }, agentPaused: false, agentDryRun: false, - installationPermissions: { pull_requests: "write", issues: "write" }, + installationPermissions: { pull_requests: "write", contents: "write", issues: "write" }, ...over, }; } @@ -443,16 +443,23 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(JSON.parse((await auditFor(env, "merge"))?.metadata_json ?? "{}")).toMatchObject({ autonomyLevel: "observe" }); }); - it("PR-write without pull_requests:write → denied (re-consent), but label still runs (issues:write)", async () => { + it("pull-request writes without pull_requests:write are denied, but label and merge use their own permissions", async () => { const env = createTestEnv({}); - const outcomes = await executeAgentMaintenanceActions(env, ctx({ installationPermissions: { pull_requests: "read", issues: "write" } }), [label, merge, updateBranch]); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ installationPermissions: { pull_requests: "read", contents: "write", issues: "write" } }), [label, merge, updateBranch]); expect(outcomes.find((o) => o.actionClass === "label")?.outcome).toBe("completed"); - expect(outcomes.find((o) => o.actionClass === "merge")?.outcome).toBe("denied"); + expect(outcomes.find((o) => o.actionClass === "merge")?.outcome).toBe("completed"); expect(outcomes.find((o) => o.actionClass === "update_branch")?.outcome).toBe("denied"); expect(ensurePullRequestLabel).toHaveBeenCalledTimes(1); - expect(mergePullRequest).not.toHaveBeenCalled(); + expect(mergePullRequest).toHaveBeenCalledTimes(1); expect(updatePullRequestBranch).not.toHaveBeenCalled(); - expect((await auditFor(env, "merge"))?.outcome).toBe("denied"); + expect((await auditFor(env, "update_branch"))?.outcome).toBe("denied"); + }); + + it("REGRESSION: merge without contents:write is denied before any GitHub mutation", async () => { + const env = createTestEnv({}); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ installationPermissions: { pull_requests: "write", contents: "read", issues: "write" } }), [merge]); + expect(outcomes[0]).toMatchObject({ actionClass: "merge", outcome: "denied", detail: "contents: write not granted — maintainer must re-consent" }); + expect(mergePullRequest).not.toHaveBeenCalled(); }); describe("write-permission denial cooldown (#selfhost-runtime-drift)", () => { @@ -463,12 +470,12 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { return Number(row?.c ?? 0); } - it("suppresses a repeated pull_requests:write denial within the cooldown window — still denied, but not re-audited", async () => { + it("suppresses a repeated write-permission denial within the cooldown window — still denied, but not re-audited", async () => { const env = createTestEnv({}); - const deniedCtx = ctx({ installationId: 200, installationPermissions: { pull_requests: "read", issues: "write" } }); + const deniedCtx = ctx({ installationId: 200, installationPermissions: { pull_requests: "write", contents: "read", issues: "write" } }); const first = await executeAgentMaintenanceActions(env, deniedCtx, [merge]); - expect(first[0]).toMatchObject({ outcome: "denied", detail: "pull_requests: write not granted — maintainer must re-consent" }); + expect(first[0]).toMatchObject({ outcome: "denied", detail: "contents: write not granted — maintainer must re-consent" }); expect(await auditCount(env, "merge")).toBe(1); const second = await executeAgentMaintenanceActions(env, deniedCtx, [merge]); @@ -484,7 +491,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("resumes loud auditing once the cooldown window elapses", async () => { const env = createTestEnv({}); - const deniedCtx = ctx({ installationId: 201, installationPermissions: { pull_requests: "read", issues: "write" } }); + const deniedCtx = ctx({ installationId: 201, installationPermissions: { pull_requests: "write", contents: "read", issues: "write" } }); vi.useFakeTimers(); vi.setSystemTime(new Date("2026-07-03T00:00:00Z")); try { @@ -497,7 +504,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { vi.setSystemTime(new Date("2026-07-03T00:15:01Z")); // cooldown elapsed const afterCooldown = await executeAgentMaintenanceActions(env, deniedCtx, [merge]); - expect(afterCooldown[0]?.detail).toBe("pull_requests: write not granted — maintainer must re-consent"); + expect(afterCooldown[0]?.detail).toBe("contents: write not granted — maintainer must re-consent"); expect(await auditCount(env, "merge")).toBe(2); } finally { vi.useRealTimers(); @@ -506,7 +513,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("scopes the cooldown per installation/repo/action-class — a different action class is not suppressed by another's denial", async () => { const env = createTestEnv({}); - const deniedCtx = ctx({ installationId: 202, installationPermissions: { pull_requests: "read", issues: "write" } }); + const deniedCtx = ctx({ installationId: 202, installationPermissions: { pull_requests: "read", contents: "read", issues: "write" } }); await executeAgentMaintenanceActions(env, deniedCtx, [merge]); const closeOutcome = await executeAgentMaintenanceActions(env, deniedCtx, [close]); @@ -517,17 +524,17 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("scopes the cooldown per repo — the SAME installation denied on a different repo is not suppressed", async () => { const env = createTestEnv({}); - const perms = { pull_requests: "read" as const, issues: "write" as const }; + const perms = { pull_requests: "write" as const, contents: "read" as const, issues: "write" as const }; await executeAgentMaintenanceActions(env, ctx({ installationId: 203, repoFullName: "owner/repo-a", installationPermissions: perms }), [merge]); const otherRepo = await executeAgentMaintenanceActions(env, ctx({ installationId: 203, repoFullName: "owner/repo-b", installationPermissions: perms }), [merge]); - expect(otherRepo[0]?.detail).toBe("pull_requests: write not granted — maintainer must re-consent"); + expect(otherRepo[0]?.detail).toBe("contents: write not granted — maintainer must re-consent"); expect(await auditCount(env, "merge")).toBe(2); // one audit row per repo }); it("REGRESSION (gate finding): a transient audit-write failure on the FIRST denial does not arm the cooldown — the retry attempts the audit again instead of being silently suppressed", async () => { const env = createTestEnv({}); - const deniedCtx = ctx({ installationId: 204, installationPermissions: { pull_requests: "read", issues: "write" } }); + const deniedCtx = ctx({ installationId: 204, installationPermissions: { pull_requests: "write", contents: "read", issues: "write" } }); const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockRejectedValueOnce(new Error("D1 write error")); await expect(executeAgentMaintenanceActions(env, deniedCtx, [merge])).rejects.toThrow("D1 write error"); @@ -537,21 +544,21 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { const retry = await executeAgentMaintenanceActions(env, deniedCtx, [merge]); // Loud, not suppressed — the cooldown was never armed by the failed first attempt. - expect(retry[0]?.detail).toBe("pull_requests: write not granted — maintainer must re-consent"); + expect(retry[0]?.detail).toBe("contents: write not granted — maintainer must re-consent"); expect(await auditCount(env, "merge")).toBe(1); }); it("REGRESSION (gate finding): scopes the cooldown per PR — a denial on a DIFFERENT PR in the same installation/repo/action-class is not suppressed", async () => { const env = createTestEnv({}); - const perms = { pull_requests: "read" as const, issues: "write" as const }; + const perms = { pull_requests: "write" as const, contents: "read" as const, issues: "write" as const }; const prA = await executeAgentMaintenanceActions(env, ctx({ installationId: 205, pullNumber: 501, installationPermissions: perms }), [merge]); const prB = await executeAgentMaintenanceActions(env, ctx({ installationId: 205, pullNumber: 502, installationPermissions: perms }), [merge]); // Both denials are loud — PR B's cooldown key differs from PR A's, so it must never be silently // suppressed by PR A's already-armed cooldown within the same 15m window. - expect(prA[0]).toMatchObject({ outcome: "denied", detail: "pull_requests: write not granted — maintainer must re-consent" }); - expect(prB[0]).toMatchObject({ outcome: "denied", detail: "pull_requests: write not granted — maintainer must re-consent" }); + expect(prA[0]).toMatchObject({ outcome: "denied", detail: "contents: write not granted — maintainer must re-consent" }); + expect(prB[0]).toMatchObject({ outcome: "denied", detail: "contents: write not granted — maintainer must re-consent" }); expect(await auditCount(env, "merge")).toBe(2); // one audit row per PR, not one shared row }); }); diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index 67494d83c2..7160729618 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -76,7 +76,7 @@ function ctx(over: Partial = {}): AgentActionExecut autonomy: { merge: "auto_with_approval" }, agentPaused: false, agentDryRun: false, - installationPermissions: { pull_requests: "write", issues: "write" }, + installationPermissions: { contents: "write", pull_requests: "write", issues: "write" }, ...over, }; } @@ -89,7 +89,7 @@ async function seedInstallation(env: Env): Promise { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", - permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"], }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], @@ -411,7 +411,7 @@ describe("agent approval queue (#779)", () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertRepositorySettings(env, { repoFullName: "solorepo", autonomy: { close: "auto_with_approval" } }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, repositories: [{ name: "solorepo", full_name: "solorepo", private: false, owner: { login: "owner" } }], }); await upsertPullRequestFromGitHub(env, "solorepo", { number: 7, title: "PR", state: "open", head: { sha: "h7" }, labels: [], body: "Closes #9" }); @@ -787,7 +787,7 @@ describe("agent approval queue (#779)", () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertRepositorySettings(env, { repoFullName: "solorepo", autonomy: { merge: "auto_with_approval" } }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, repositories: [{ name: "solorepo", full_name: "solorepo", private: false, owner: { login: "owner" } }], }); // No `user` on the payload → authorLogin stored null; repoFullName has no "/" → repoOwner falls back to "". diff --git a/test/unit/agent-execution.test.ts b/test/unit/agent-execution.test.ts index 949537ada1..5bb25bf2ba 100644 --- a/test/unit/agent-execution.test.ts +++ b/test/unit/agent-execution.test.ts @@ -1,9 +1,12 @@ import { describe, expect, it } from "vitest"; import { agentActionModeExecutes, + agentRequiresContentsWrite, agentRequiresPrWrite, buildAgentActionAudit, + formatAgentPermissionDenial, isGlobalAgentPause, + requiredAgentActionPermissions, resolveAgentActionMode, resolveAgentPermissionReadiness, } from "../../src/settings/agent-execution"; @@ -75,7 +78,8 @@ describe("buildAgentActionAudit", () => { describe("agent write-permission readiness (#775)", () => { it("agentRequiresPrWrite is true only for an acting level on a PR-write action class", () => { - expect(agentRequiresPrWrite({ merge: "auto" })).toBe(true); + expect(agentRequiresPrWrite({ merge: "auto" })).toBe(false); + expect(agentRequiresContentsWrite({ merge: "auto" })).toBe(true); expect(agentRequiresPrWrite({ request_changes: "auto_with_approval" })).toBe(true); expect(agentRequiresPrWrite({ close: "auto" })).toBe(true); // non-acting levels never demand write @@ -89,14 +93,16 @@ describe("agent write-permission readiness (#775)", () => { expect(agentRequiresPrWrite({ update_branch: "observe" })).toBe(false); // non-acting still no write // label acts via the Issues API (issues: write, already held), so it does NOT demand pull_requests: write expect(agentRequiresPrWrite({ label: "auto" })).toBe(false); + expect(agentRequiresContentsWrite({ merge: "observe" })).toBe(false); + expect(agentRequiresContentsWrite({})).toBe(false); + expect(agentRequiresContentsWrite(null)).toBe(false); }); - it("INVARIANT: every executor PR_WRITE_CLASS is counted by agentRequiresPrWrite (the readiness gate cannot disagree with the runtime guard)", () => { - // The executor denies a PR-write action whose readiness !== "ready". If a class it gates were missing from - // PR_WRITE_ACTION_CLASSES, agentRequiresPrWrite would grade it "not_required" and the gate would diverge — the - // exact #audit-update-branch bug. Enforce executor PR_WRITE_CLASSES ⊆ readiness write-classes for ALL members. + it("INVARIANT: every executor write class has an action-specific permission requirement", () => { + // The executor denies a write action whose readiness !== "ready". If a class it gates were missing from + // requiredAgentActionPermissions, the readiness guard would grade it "not_required" and the gate would diverge. for (const actionClass of PR_WRITE_CLASSES) { - expect(agentRequiresPrWrite({ [actionClass]: "auto" })).toBe(true); + expect(requiredAgentActionPermissions({ [actionClass]: "auto" }, actionClass).length).toBeGreaterThan(0); } }); @@ -107,14 +113,25 @@ describe("agent write-permission readiness (#775)", () => { expect(resolveAgentPermissionReadiness({ autonomy: { update_branch: "auto" }, installationPermissions: { pull_requests: "read" } })).toBe("reconsent_required"); }); - it("resolveAgentPermissionReadiness gates on the granted pull_requests scope", () => { + it("resolveAgentPermissionReadiness gates on the exact granted scopes for each acting action", () => { // no acting PR-write level → permission is irrelevant expect(resolveAgentPermissionReadiness({ autonomy: { label: "auto" }, installationPermissions: { pull_requests: "read" } })).toBe("not_required"); - // acting level + write granted → ready - expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: { pull_requests: "write", issues: "write" } })).toBe("ready"); - // acting level but only read (or missing) → re-consent required - expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: { pull_requests: "read" } })).toBe("reconsent_required"); + // merge is authorized by Contents: write, not Pull requests: write. + expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: { contents: "write", pull_requests: "read" } })).toBe("ready"); + expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: { pull_requests: "write" } })).toBe("reconsent_required"); + // non-merge PR state mutations still require Pull requests: write. + expect(resolveAgentPermissionReadiness({ autonomy: { approve: "auto" }, installationPermissions: { contents: "write", pull_requests: "read" } })).toBe("reconsent_required"); + expect(resolveAgentPermissionReadiness({ autonomy: { approve: "auto" }, installationPermissions: { pull_requests: "write" } })).toBe("ready"); expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: {} })).toBe("reconsent_required"); expect(resolveAgentPermissionReadiness({ autonomy: { merge: "auto" }, installationPermissions: null })).toBe("reconsent_required"); }); + + it("formats the missing action permission instead of blaming pull_requests for merge", () => { + expect(formatAgentPermissionDenial({ autonomy: { merge: "auto" }, installationPermissions: { pull_requests: "write" }, actionClass: "merge" })).toBe( + "contents: write not granted — maintainer must re-consent", + ); + expect(formatAgentPermissionDenial({ autonomy: { close: "auto" }, installationPermissions: { contents: "write" }, actionClass: "close", suppressed: true })).toBe( + "pull_requests: write not granted — maintainer must re-consent (suppressed repeat)", + ); + }); }); diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index faaf5d4faf..8af24f8401 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -501,6 +501,43 @@ describe("GitHub backfill", () => { expect(await getInstallationHealth(env, 900)).toMatchObject({ authMode: "broker" }); }); + it("REGRESSION: broker-mode refresh replaces stale local permissions with the broker token permission snapshot", async () => { + const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_test" }); + await upsertInstallation(env, { + installation: { + id: 912, + account: { login: "brokered-owner", id: 9, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "read" }, + }, + }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 912); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { merge: "auto" } }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.endsWith("/v1/orb/token")) { + return Response.json({ + token: "ghs_brokered", + installationId: 912, + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "write" }, + }); + } + return new Response("not found", { status: 404 }); + }); + + const result = await refreshInstallationHealth(env); + + expect(result.installations[0]).toMatchObject({ + status: "healthy", + authMode: "broker", + missingPermissions: [], + }); + expect(await getInstallationHealth(env, 912)).toMatchObject({ + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "write" }, + missingPermissions: [], + }); + }); + it("REGRESSION (gate finding): never reports healthy when the broker mints a token for a DIFFERENT installation than the one being refreshed", async () => { const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_test" }); // Two local rows exist (e.g. a stale row left over from a prior re-registration), but a brokered @@ -607,6 +644,27 @@ describe("GitHub backfill", () => { ); expect(healthy.repairSteps.join(" ")).toMatch(/token broker is reachable/i); + const staleSnapshot = enrichInstallationHealth({ + installationId: 905, + accountLogin: "brokered-owner", + repositorySelection: "selected", + installedReposCount: 1, + registeredInstalledCount: 1, + status: "needs_attention", + missingPermissions: ["contents"], + missingEvents: [], + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "read" }, + events: [], + checkedAt: "2026-07-03T00:00:00.000Z", + authMode: "broker", + }); + expect(staleSnapshot.requiredPermissions).toMatchObject({ contents: "write" }); + expect(staleSnapshot.permissionRemediation).toEqual( + expect.arrayContaining([ + expect.objectContaining({ permission: "contents", requiredAccess: "write", currentAccess: "read", ok: false }), + ]), + ); + const degraded = enrichInstallationHealth({ installationId: 903, accountLogin: "brokered-owner", @@ -800,6 +858,48 @@ describe("GitHub backfill", () => { ); }); + it("REGRESSION: merge autonomy requires contents:write, so contents:read is needs_attention before merge 403s", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedRegisteredRepo(env); + await upsertInstallation(env, { + installation: { + id: 125, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "read" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }, + }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 125); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { merge: "auto" } }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.endsWith("/app/installations/125")) { + return Response.json({ + id: 125, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "read" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }); + } + return new Response("not found", { status: 404 }); + }); + + const refreshed = await refreshInstallationHealth(env); + + expect(refreshed.installations).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + installationId: 125, + status: "needs_attention", + missingPermissions: ["contents"], + requiredPermissions: expect.objectContaining({ pull_requests: "read", contents: "write" }), + }), + ]), + ); + }); + it("marks comment, label, and check repair impacts disabled by repo settings", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 123); @@ -828,6 +928,7 @@ describe("GitHub backfill", () => { expect(repair.repairSteps).toEqual(["No repair needed."]); expect(repair.requiredPermissions).not.toHaveProperty("checks"); + expect(repair.requiredPermissions).not.toHaveProperty("contents"); expect(repair.requiredPermissions.pull_requests).toBe("read"); // non-acting → baseline read, NOT upgraded to write expect(repair.modeImpacts).toEqual( expect.arrayContaining([ @@ -838,7 +939,7 @@ describe("GitHub backfill", () => { ); }); - it("repair diagnostics upgrade pull_requests to write for an acting autonomy (#audit-install-health display)", async () => { + it("repair diagnostics require contents:write for merge autonomy (#audit-install-health display)", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 123); await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { merge: "auto" } }); @@ -850,15 +951,21 @@ describe("GitHub backfill", () => { installedReposCount: 1, registeredInstalledCount: 0, status: "needs_attention", - missingPermissions: ["pull_requests"], + missingPermissions: ["contents"], missingEvents: [], - permissions: { metadata: "read", pull_requests: "read", issues: "write" }, + permissions: { metadata: "read", pull_requests: "read", issues: "write", contents: "read" }, events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], checkedAt: "2026-05-28T00:00:00.000Z", authMode: "local", }); - expect(repair.requiredPermissions.pull_requests).toBe("write"); + expect(repair.requiredPermissions.pull_requests).toBe("read"); + expect(repair.requiredPermissions.contents).toBe("write"); + expect(repair.modeImpacts).toEqual( + expect.arrayContaining([ + expect.objectContaining({ mode: "agent_merge", enabled: true, requiredPermissions: [expect.objectContaining({ permission: "contents", missing: true })] }), + ]), + ); }); it("counts comment-only and label-only repair surfaces separately", async () => { diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index 347f20ae18..3e1e54a1cc 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -26,6 +26,7 @@ import { } from "../../src/github/app"; import type { Advisory } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; +import { getInstallation, upsertInstallation } from "../../src/db/repositories"; beforeEach(() => clearInstallationTokenCacheForTest()); @@ -470,6 +471,7 @@ describe("GitHub check runs", () => { token: "brokered-token", installationId: 999, expiresAt: new Date(Date.now() + 60 * 60_000).toISOString(), + permissions: { contents: "write" }, }); } return new Response("not found", { status: 404 }); @@ -481,6 +483,69 @@ describe("GitHub check runs", () => { expect(brokerCalls).toBe(1); }); + it("persists broker-returned installation permissions so runtime readiness sees accepted scopes", async () => { + const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_test" }); + await upsertInstallation(env, { + installation: { + id: 889, + account: { login: "owner", id: 1, type: "User" }, + target_type: "User", + repository_selection: "selected", + permissions: { contents: "read", pull_requests: "write" }, + events: [], + }, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + if (input.toString().includes("/v1/orb/token")) { + return Response.json({ + token: "brokered-token", + installationId: 889, + expiresAt: new Date(Date.now() + 60 * 60_000).toISOString(), + permissions: { contents: "write", pull_requests: "write" }, + }); + } + return new Response("not found", { status: 404 }); + }); + + await createInstallationToken(env, 889); + + expect((await getInstallation(env, 889))?.permissions).toEqual({ contents: "write", pull_requests: "write" }); + }); + + it("force-remints the broker token once when GitHub rejects a stale permission scope", async () => { + const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_test" }); + const brokerBodies: Array = []; + let brokerCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + if (input.toString().includes("/v1/orb/token")) { + brokerCalls += 1; + brokerBodies.push(init?.body === undefined ? undefined : String(init.body)); + return Response.json({ + token: brokerCalls === 1 ? "stale-scope-token" : "fresh-scope-token", + installationId: 890, + expiresAt: new Date(Date.now() + 60 * 60_000).toISOString(), + permissions: brokerCalls === 1 ? { contents: "read" } : { contents: "write" }, + }); + } + return new Response("not found", { status: 404 }); + }); + const seenTokens: string[] = []; + + const result = await withInstallationTokenRetry(env, 890, async (token) => { + seenTokens.push(token); + if (token === "stale-scope-token") { + const error = new Error("Resource not accessible by integration") as Error & { status: number }; + error.status = 403; + throw error; + } + return "ok"; + }); + + expect(result).toBe("ok"); + expect(seenTokens).toEqual(["stale-scope-token", "fresh-scope-token"]); + expect(brokerBodies).toEqual([undefined, JSON.stringify({ forceRefresh: true })]); + }); + it("#2: serves a still-valid cached token when the Orb mint fails (stale-token grace, no fleet stall)", async () => { let calls = 0; vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index c5669193f1..5cfec5efd2 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -77,7 +77,7 @@ describe("MCP gittensory_get_automation_state (#784)", () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", label: "auto_with_approval" }, agentDryRun: true }); @@ -89,7 +89,7 @@ describe("MCP gittensory_get_automation_state (#784)", () => { const data = result.structuredContent as State; expect(data.configured).toBe(true); expect(data.mode).toBe("dry_run"); // agentDryRun → dry_run - expect(data.permissionReadiness).toBe("ready"); // pull_requests: write granted + expect(data.permissionReadiness).toBe("ready"); // contents: write granted for merge; pull_requests: write granted for PR writes expect(data.actingActionClasses).toEqual(expect.arrayContaining(["merge", "label"])); expect(data.pendingActionCount).toBe(1); // surfaces the COUNT, not the queue details — no reward/wallet leakage either @@ -171,7 +171,7 @@ describe("MCP gittensory_propose_action (#784)", () => { it("an MCP-staged merge is superseded on accept if the PR is force-pushed after proposal — the guard now actually fires (#2255)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); @@ -193,7 +193,7 @@ describe("MCP gittensory_propose_action (#784)", () => { it("allows a session that maintains the repo (owned installation)", async () => { const env = createTestEnv(); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); @@ -267,7 +267,7 @@ describe("MCP gittensory_propose_action (#784)", () => { it("does not trust cached collaborator association without live write permission", async () => { const env = createTestEnv(); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); @@ -397,7 +397,7 @@ describe("MCP gittensory_decide_pending_action (#784)", () => { it("accepts a staged action and honors dry-run mode (no live mutation)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); @@ -416,7 +416,7 @@ describe("MCP gittensory_decide_pending_action (#784)", () => { it("REGRESSION (#2423): reports status=errored (not accepted) and a distinct summary when the live mutation throws", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); diff --git a/test/unit/opportunity-branch-internals.test.ts b/test/unit/opportunity-branch-internals.test.ts index b71a57d7f9..208d2c1c60 100644 --- a/test/unit/opportunity-branch-internals.test.ts +++ b/test/unit/opportunity-branch-internals.test.ts @@ -40,8 +40,8 @@ describe("opportunity branch internals", () => { it("issueAgeDays floors invalid timestamps to stale age", () => { const { issueAgeDays } = opportunityFreshnessInternals; - expect(issueAgeDays(null, NOW)).toBe(9999); - expect(issueAgeDays("not-a-date", NOW)).toBe(9999); + expect(issueAgeDays(null, NOW)).toBe(Number.POSITIVE_INFINITY); + expect(issueAgeDays("not-a-date", NOW)).toBe(Number.POSITIVE_INFINITY); expect(issueAgeDays("2026-07-03T00:00:00.000Z", NOW)).toBeGreaterThanOrEqual(0); }); diff --git a/test/unit/orb-app-auth.test.ts b/test/unit/orb-app-auth.test.ts index 60ff6e35be..e0e929a3df 100644 --- a/test/unit/orb-app-auth.test.ts +++ b/test/unit/orb-app-auth.test.ts @@ -45,10 +45,10 @@ describe("createOrbInstallationToken", () => { const env = async (): Promise => orbEnv({ ORB_GITHUB_APP_PRIVATE_KEY: await pkcs8Pem() }); it("returns the minted token + GitHub's real expiry (empty only when absent)", async () => { - vi.stubGlobal("fetch", async () => Response.json({ token: "ghs_minted", expires_at: "2026-06-25T07:00:00Z" })); - expect(await createOrbInstallationToken(await env(), 42)).toEqual({ token: "ghs_minted", expiresAt: "2026-06-25T07:00:00Z" }); + vi.stubGlobal("fetch", async () => Response.json({ token: "ghs_minted", expires_at: "2026-06-25T07:00:00Z", permissions: { contents: "write" } })); + expect(await createOrbInstallationToken(await env(), 42)).toEqual({ token: "ghs_minted", expiresAt: "2026-06-25T07:00:00Z", permissions: { contents: "write" } }); vi.stubGlobal("fetch", async () => Response.json({ token: "ghs_noexp" })); - expect((await createOrbInstallationToken(await env(), 42)).expiresAt).toBe(""); + expect(await createOrbInstallationToken(await env(), 42)).toMatchObject({ expiresAt: "", permissions: {} }); }); it("throws on a non-ok response or a missing token", async () => { diff --git a/test/unit/orb-broker-client.test.ts b/test/unit/orb-broker-client.test.ts index c9cc429605..9bfd6644f7 100644 --- a/test/unit/orb-broker-client.test.ts +++ b/test/unit/orb-broker-client.test.ts @@ -28,12 +28,20 @@ describe("isOrbBrokerMode", () => { describe("fetchBrokeredInstallationToken", () => { it("exchanges the secret for a token + parses the expiry (default broker URL + Bearer secret)", async () => { - const { fetchImpl, calls } = captureFetch(Response.json({ token: "ghs_x", installationId: 42, expiresAt: "2026-06-25T09:00:00Z" })); + const { fetchImpl, calls } = captureFetch(Response.json({ token: "ghs_x", installationId: 42, expiresAt: "2026-06-25T09:00:00Z", permissions: { contents: "write" } })); const out = await fetchBrokeredInstallationToken({ ORB_ENROLLMENT_SECRET: "orbsec_x" }, fetchImpl); - expect(out).toEqual({ token: "ghs_x", installationId: 42, expiresAtMs: Date.parse("2026-06-25T09:00:00Z") }); + expect(out).toEqual({ token: "ghs_x", installationId: 42, expiresAtMs: Date.parse("2026-06-25T09:00:00Z"), permissions: { contents: "write" } }); expect(calls[0]?.url).toBe("https://gittensory-api.aethereal.dev/v1/orb/token"); expect((calls[0]?.init?.headers as Record).authorization).toBe("Bearer orbsec_x"); expect(calls[0]?.init?.method).toBe("POST"); + expect(calls[0]?.init?.body).toBeUndefined(); + }); + + it("asks the broker to force-refresh when retrying a stale permission scope", async () => { + const { fetchImpl, calls } = captureFetch(Response.json({ token: "ghs_x", installationId: 42, expiresAt: "2026-06-25T09:00:00Z" })); + await fetchBrokeredInstallationToken({ ORB_ENROLLMENT_SECRET: "orbsec_x" }, fetchImpl, { forceRefresh: true }); + expect((calls[0]?.init?.headers as Record)["content-type"]).toBe("application/json"); + expect(JSON.parse(String(calls[0]?.init?.body))).toEqual({ forceRefresh: true }); }); it("defaults installationId + expiry when absent, and strips a trailing slash from a custom broker URL", async () => { @@ -41,6 +49,7 @@ describe("fetchBrokeredInstallationToken", () => { const out = await fetchBrokeredInstallationToken({ ORB_ENROLLMENT_SECRET: "s", ORB_BROKER_URL: "https://broker.example/" }, fetchImpl); expect(out.token).toBe("ghs_y"); expect(out.installationId).toBe(0); // payload.installationId ?? 0 + expect(out.permissions).toEqual({}); expect(out.expiresAtMs).toBeGreaterThan(Date.now()); // payload.expiresAt absent → ~50min default expect(calls[0]?.url).toBe("https://broker.example/v1/orb/token"); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 1899845a5b..7718636117 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -5216,7 +5216,7 @@ describe("queue processors", () => { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", - permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], }, repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }], @@ -6079,7 +6079,7 @@ describe("queue processors", () => { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", - permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"], }, repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], @@ -7285,7 +7285,7 @@ describe("queue processors", () => { async function seedMigrationRecheckRepo(env: Env, prNumber: number, opts: { premergeContentRecheck?: boolean } = {}) { await upsertInstallation(env, { - installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write" }, events: [] }, + installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { contents: "write", pull_requests: "write", issues: "write" }, events: [] }, }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto", review_state_label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); @@ -7363,7 +7363,7 @@ describe("queue processors", () => { it("fails OPEN (never fetches the live tree) when the PR has no resolvable base ref", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertInstallation(env, { - installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write" }, events: [] }, + installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { contents: "write", pull_requests: "write", issues: "write" }, events: [] }, }); // No default_branch on the repo record AND no base.ref on the PR record — baseRef resolves to undefined. await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); @@ -7731,7 +7731,7 @@ describe("queue processors", () => { async function seedFreshRebaseRepo(env: Env, prNumber: number, opts: { requireFreshRebaseWindowMinutes?: number | null; autonomy?: Record } = {}) { await upsertInstallation(env, { - installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write" }, events: [] }, + installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { contents: "write", pull_requests: "write", issues: "write" }, events: [] }, }); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); await upsertRepositorySettings(env, { @@ -17482,7 +17482,7 @@ describe("agentMaintenanceHeadMatchesGate", () => { account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", - permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"], }, repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], @@ -19313,7 +19313,7 @@ describe("auto-action convergence: end-to-end plan+execute for the general heuri id: INSTALLATION_ID, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", - permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"], }, repositories: [{ name: "gittensory", full_name: REPO, private: false, owner: { login: "JSONbored" } }], diff --git a/test/unit/routes-agent-approval.test.ts b/test/unit/routes-agent-approval.test.ts index ff8813d8d3..04d8abf1ae 100644 --- a/test/unit/routes-agent-approval.test.ts +++ b/test/unit/routes-agent-approval.test.ts @@ -46,7 +46,7 @@ const headers = (env: Env) => ({ authorization: `Bearer ${env.GITTENSORY_API_TOK async function seedPending(env: Env) { await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); await upsertInstallation(env, { - installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], }); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); diff --git a/test/unit/settings-preview.test.ts b/test/unit/settings-preview.test.ts index 2c8a181c61..e2b1d789fe 100644 --- a/test/unit/settings-preview.test.ts +++ b/test/unit/settings-preview.test.ts @@ -394,4 +394,16 @@ describe("buildRepoSettingsPreview", () => { const actualReadPerms = (preview.installPreview.permissions.required as string[]).filter((p) => p.endsWith(": read")).sort(); expect(actualReadPerms).toEqual(expectedReadPerms); }); + + it("REGRESSION: merge autonomy requires contents: write in the install preview", () => { + const preview = buildRepoSettingsPreview({ + ...base, + settings: settings({ publicSurface: "label_only", autoLabelEnabled: false, commentMode: "off", checkRunMode: "off", autonomy: { merge: "auto" } }), + installation: { ...healthyInstall, missingPermissions: ["contents"] }, + sample: { authorLogin: "miner", minerStatus: "confirmed" }, + }); + + expect(preview.installPreview.permissions.required).toContain("contents: write"); + expect(preview.installPreview.permissions.missing).toContain("contents"); + }); });