diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 681e6a905b..927e9ac448 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -252,6 +252,12 @@ settings: mergeMethod: squash requireApprovals: 1 - # Command authorization role policy. Map. Default: the built-in policy. - # Omit to inherit the safe built-in default. - # commandAuthorization: {} + # Command authorization role policy — which roles (maintainer | collaborator | pr_author | + # confirmed_miner) may invoke each bot command. Map. Default: the built-in policy. + # Omit to inherit the DB-stored policy, or the built-in default if neither is set. An + # invalid top-level shape (not a mapping) is ignored with a warning rather than + # overwriting an existing DB-stored policy with the built-in default. + # commandAuthorization: + # default: [maintainer, collaborator, confirmed_miner] + # commands: + # gate-override: [maintainer, collaborator] diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index ca51f5038a..066126dbb3 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -1,6 +1,7 @@ import { parse as parseYaml } from "yaml"; import type { GatePolicyPack, GateRuleMode, JsonValue, RepositorySettings } from "../types"; import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy } from "../settings/autonomy"; +import { normalizeCommandAuthorizationPolicy } from "../settings/command-authorization"; import { mergeContributorBlacklists, normalizeContributorBlacklist } from "../settings/contributor-blacklist"; import { PUBLIC_LOCAL_PATH_INLINE } from "./redaction"; @@ -98,6 +99,7 @@ export type FocusManifestSettings = Partial< | "autoMaintain" | "agentPaused" | "agentDryRun" + | "commandAuthorization" | "contributorBlacklist" | "blacklistLabel" > @@ -641,6 +643,22 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) if (typeof r.autoMaintain === "object" && r.autoMaintain !== null && !Array.isArray(r.autoMaintain)) { out.autoMaintain = normalizeAutoMaintainPolicy(r.autoMaintain); } + // Command authorization policy (#2268 config-as-code parity): `settings.commandAuthorization` declares the + // full role policy the same way `autoMaintain` does — the normalizer fills any unset/invalid FIELD from + // DEFAULT_COMMAND_AUTHORIZATION_POLICY, so a partially-valid mapping yields a complete, safe policy that + // overlays the DB value via the resolver's `{...dbSettings, ...manifest.settings}` spread. But an invalid + // TOP-LEVEL shape (not a mapping at all) is a different case: normalizeCommandAuthorizationPolicy's own + // fallback there is meant for callers with no DB value to fall back to, not for this overlay — applying it + // here would let a typo'd config silently overwrite a stricter DB-persisted policy with the built-in + // default. So only apply the normalized policy when the raw value was actually a mapping; otherwise warn + // and leave `out.commandAuthorization` unset so the resolver preserves whatever the DB already has. + if (typeof r.commandAuthorization === "object" && r.commandAuthorization !== null && !Array.isArray(r.commandAuthorization)) { + const { policy, warnings: commandAuthorizationWarnings } = normalizeCommandAuthorizationPolicy(r.commandAuthorization); + warnings.push(...commandAuthorizationWarnings); + out.commandAuthorization = policy; + } else if (r.commandAuthorization !== undefined) { + warnings.push(`Manifest "settings.commandAuthorization" must be an object; ignoring it and keeping any existing policy.`); + } // Contributor blacklist (#1425): `settings.contributorBlacklist` is a list of banned-login entries. Only set it // when at least one VALID entry survives normalization, so a malformed block never blanks the DB-configured // list via the resolver's `{...dbSettings, ...manifest.settings}` overlay. Normalization warnings are folded in. diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 9de5e943be..ee2bf488b7 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -19,6 +19,7 @@ import { settingsOverrideToJson, type FocusManifest, } from "../../src/signals/focus-manifest"; +import { DEFAULT_COMMAND_AUTHORIZATION_POLICY } from "../../src/settings/command-authorization"; import type { RepositorySettings } from "../../src/types"; const FULL_MANIFEST = { @@ -1143,6 +1144,51 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(ignored.autoMaintain).toEqual({ requireApprovals: 2, mergeMethod: "merge" }); }); + it("parses + resolves commandAuthorization from the settings: block, overlaying the DB (#2268)", () => { + const manifest = parseFocusManifest({ settings: { commandAuthorization: { commands: { "gate-override": ["maintainer"] } } } }); + expect(manifest.settings.commandAuthorization).toEqual({ + ...DEFAULT_COMMAND_AUTHORIZATION_POLICY, + commands: { ...DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands, "gate-override": ["maintainer"] }, + }); + expect(manifest.warnings.some((w) => /commandAuthorization/.test(w))).toBe(false); + + const dbPolicy = { default: ["maintainer", "collaborator", "confirmed_miner", "pr_author"], commands: {} } as RepositorySettings["commandAuthorization"]; + const eff = resolveEffectiveSettings({ commandAuthorization: dbPolicy } as unknown as RepositorySettings, manifest); + expect(eff.commandAuthorization?.commands["gate-override"]).toEqual(["maintainer"]); // yml overlays DB + + // Unset key means "no opinion" and must leave the DB-stored policy untouched — never reset to defaults. + const noOverride = resolveEffectiveSettings({ commandAuthorization: dbPolicy } as unknown as RepositorySettings, parseFocusManifest({ settings: { commentMode: "off" } })); + expect(noOverride.commandAuthorization).toEqual(dbPolicy); + }); + + it("ignores an invalid top-level commandAuthorization shape with a visible warning, never overwriting the DB policy (#2268)", () => { + const manifest = parseFocusManifest({ settings: { commandAuthorization: "nope" } }); + expect(manifest.settings.commandAuthorization).toBeUndefined(); + expect(manifest.warnings.some((w) => /commandAuthorization.*must be an object/.test(w))).toBe(true); + + // A malformed shape must leave the DB-persisted policy intact via the resolver overlay — never reset to + // the built-in default, which could be less restrictive than what the DB has on record. + const dbPolicy = { default: ["maintainer"], commands: { "gate-override": ["maintainer"] } } as RepositorySettings["commandAuthorization"]; + const eff = resolveEffectiveSettings({ commandAuthorization: dbPolicy } as unknown as RepositorySettings, manifest); + expect(eff.commandAuthorization).toEqual(dbPolicy); + + // A null value is likewise rejected (typeof null === "object" but it is not a valid mapping). + const nullShape = parseFocusManifest({ settings: { commandAuthorization: null } }); + expect(nullShape.settings.commandAuthorization).toBeUndefined(); + expect(nullShape.warnings.some((w) => /commandAuthorization.*must be an object/.test(w))).toBe(true); + + // An array is likewise rejected, not treated as a mapping. + const arrayShape = parseFocusManifest({ settings: { commandAuthorization: ["nope"] } }); + expect(arrayShape.settings.commandAuthorization).toBeUndefined(); + expect(arrayShape.warnings.some((w) => /commandAuthorization.*must be an object/.test(w))).toBe(true); + + // A spoofable role on a maintainer-only command is clamped back to the default for that command, not + // dropped silently — the maintainer-only invariant holds even inside a partially-valid override. + const badRole = parseFocusManifest({ settings: { commandAuthorization: { commands: { "gate-override": ["pr_author"] } } } }); + expect(badRole.settings.commandAuthorization?.commands["gate-override"]).toEqual(["maintainer", "collaborator"]); + expect(badRole.warnings.some((w) => /maintainer-only command/.test(w))).toBe(true); + }); + it("parses + resolves contributorBlacklist + blacklistLabel from the settings: block, overlaying the DB (#1425)", () => { const manifest = parseFocusManifest({ settings: { contributorBlacklist: ["plagiarist1", { login: "farmer2", reason: "farming" }, { login: "-bad" }], blacklistLabel: "abuse" } }); expect(manifest.settings.contributorBlacklist).toEqual([{ login: "plagiarist1" }, { login: "farmer2", reason: "farming" }]); // invalid login dropped