From 55e845df58d375b179bfeb1c5a2e2bca75742870 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 07:04:27 -0700 Subject: [PATCH 1/3] fix(config): honor settings.commandAuthorization in .gittensory.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FocusManifestSettings never included commandAuthorization and parseSettingsOverride never read it, so a maintainer setting settings.commandAuthorization in .gittensory.yml had it silently dropped — the DB-stored (possibly more permissive) policy stayed in effect with no warning, contradicting this repo's yml > DB > defaults config-as-code convention for every other settings field. Add commandAuthorization to FocusManifestSettings and parse it with the existing normalizeCommandAuthorizationPolicy, folding its warnings into the manifest output. The normalizer always fills unset or invalid sub-fields from the secure built-in default, so once the key is present in yml it always yields a complete policy that overlays the DB value the same way autoMaintain already does. --- .gittensory.yml.example | 12 +++++++++--- src/signals/focus-manifest.ts | 13 +++++++++++++ test/unit/focus-manifest.test.ts | 30 ++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/.gittensory.yml.example b/.gittensory.yml.example index aaed6cc35d..3769cbba7d 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -247,6 +247,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 shape falls back to the built-in default with a warning rather than being + # silently dropped. + # 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 1cf6fe245d..260cce5032 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" > @@ -616,6 +618,17 @@ 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 always fills any unset/invalid field + // from DEFAULT_COMMAND_AUTHORIZATION_POLICY (the same secure default used when the DB has none), so once + // the key is present in yml it always yields a complete, safe policy that overlays the DB value via the + // resolver's `{...dbSettings, ...manifest.settings}` spread. Normalization warnings are folded in so an + // invalid shape is visible rather than silently dropped. + if (r.commandAuthorization !== undefined) { + const { policy, warnings: commandAuthorizationWarnings } = normalizeCommandAuthorizationPolicy(r.commandAuthorization); + warnings.push(...commandAuthorizationWarnings); + out.commandAuthorization = 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 bfc420ec20..5b552fb9de 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 = { @@ -1099,6 +1100,35 @@ 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("falls back to the secure built-in default with a visible warning on an invalid commandAuthorization shape (#2268)", () => { + const manifest = parseFocusManifest({ settings: { commandAuthorization: "nope" } }); + expect(manifest.settings.commandAuthorization).toEqual(DEFAULT_COMMAND_AUTHORIZATION_POLICY); + expect(manifest.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 From 86a3a5efab1bbc3353fa1429ae3a1c9ec74e7cc5 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:48:07 -0700 Subject: [PATCH 2/3] fix(config): never let an invalid commandAuthorization shape overwrite the DB policy normalizeCommandAuthorizationPolicy falls back to the built-in default policy (with a warning) when the top-level shape isn't a mapping, but focus-manifest.ts unconditionally wrote that fallback result into the parsed settings override. Since the resolver overlays manifest settings on top of DB settings, a malformed .gittensory.yml value (e.g. a string instead of an object) silently overwrote a stricter DB-persisted commandAuthorization policy with the built-in default. Only apply the normalized policy when the raw value is actually a mapping; otherwise warn and leave the override unset so the resolver preserves whatever the DB already has. --- src/signals/focus-manifest.ts | 17 +++++++++++------ test/unit/focus-manifest.test.ts | 22 +++++++++++++++++++--- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 260cce5032..8e3063de0c 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -619,15 +619,20 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[]) 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 always fills any unset/invalid field - // from DEFAULT_COMMAND_AUTHORIZATION_POLICY (the same secure default used when the DB has none), so once - // the key is present in yml it always yields a complete, safe policy that overlays the DB value via the - // resolver's `{...dbSettings, ...manifest.settings}` spread. Normalization warnings are folded in so an - // invalid shape is visible rather than silently dropped. - if (r.commandAuthorization !== undefined) { + // 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 diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 5b552fb9de..ca11e1268c 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1117,10 +1117,26 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(noOverride.commandAuthorization).toEqual(dbPolicy); }); - it("falls back to the secure built-in default with a visible warning on an invalid commandAuthorization shape (#2268)", () => { + 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).toEqual(DEFAULT_COMMAND_AUTHORIZATION_POLICY); - expect(manifest.warnings.some((w) => /commandAuthorization must be an object/.test(w))).toBe(true); + 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. From dce7d9a80881cf5b22808ae33de0d5aea4985269 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:56:10 -0700 Subject: [PATCH 3/3] docs(config): correct the commandAuthorization invalid-shape contract in the example The example still described the pre-fix behavior (invalid shape falls back to the built-in default). Update it to match the actual contract: an invalid top-level shape is ignored with a warning, preserving any existing DB-stored policy instead of overwriting it. --- .gittensory.yml.example | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 3260cf9007..927e9ac448 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -255,8 +255,8 @@ settings: # 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 shape falls back to the built-in default with a warning rather than being - # silently dropped. + # 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: