fix(engine): deep-copy and freeze the default command-authorization policy

packages/loopover-engine/src/settings/command-authorization.ts

@@ -43,6 +43,15 @@ export const DEFAULT_COMMAND_AUTHORIZATION_POLICY: RepositoryCommandAuthorizatio
   },
 };
 
+// #9998: freeze the security vocabulary at runtime -- the object, its `default` array, its `commands` record,
+// and every role array inside it -- so a future aliasing regression fails loudly (a strict-mode TypeError on
+// the offending `push`) instead of silently widening a command for every repo in the isolate. Values are
+// unchanged; normalizeCommandAuthorizationPolicy always hands callers a fresh deep copy to mutate.
+for (const roles of Object.values(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands)) Object.freeze(roles);
+Object.freeze(DEFAULT_COMMAND_AUTHORIZATION_POLICY.default);
+Object.freeze(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands);
+Object.freeze(DEFAULT_COMMAND_AUTHORIZATION_POLICY);
+
 const COMMAND_AUTHORIZATION_ROLES = new Set<CommandAuthorizationRole>(["maintainer", "collaborator", "pr_author", "confirmed_miner"]);
 // Roles that may remain configured on a maintainer-only command. The clamp drops only the spoofable
 // plain `pr_author` role; `confirmed_miner` survives so a detected miner can self-trigger reruns (#824).
@@ -70,7 +79,12 @@ export function normalizeCommandAuthorizationPolicy(input: unknown): { policy: R
   }
 
   const defaultRoles = normalizeRoleList(input.default, DEFAULT_COMMAND_AUTHORIZATION_POLICY.default, "default", warnings);
-  const commands: Record<string, CommandAuthorizationRole[]> = { ...DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands };
+  // #9998: DEEP-copy the default command arrays, not a shallow spread. A shallow `{ ...DEFAULT...commands }`
+  // shares every un-overridden command's role array with the module-level (now frozen) default, so a caller
+  // that mutated a returned array would corrupt the security vocabulary for every repo in the isolate. This
+  // reuses `clonePolicy` -- the same deep copy the non-record exit already returns -- so both paths hand back
+  // arrays the caller solely owns.
+  const commands: Record<string, CommandAuthorizationRole[]> = clonePolicy(DEFAULT_COMMAND_AUTHORIZATION_POLICY).commands;
   if (input.commands !== undefined) {
     if (isRecord(input.commands)) {
       for (const [command, roles] of Object.entries(input.commands)) {

packages/loopover-engine/test/command-authorization.test.ts

@@ -0,0 +1,47 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import {
+  commandAuthorizationAllowedRoles,
+  DEFAULT_COMMAND_AUTHORIZATION_POLICY,
+  evaluateCommandAuthorization,
+  normalizeCommandAuthorizationPolicy,
+} from "../dist/settings/command-authorization.js";
+
+// #9998: the record path seeded `commands` with a SHALLOW spread of DEFAULT_COMMAND_AUTHORIZATION_POLICY,
+// so every un-overridden command's role array was the same instance the module-level default holds. A caller
+// that mutated a returned array would corrupt the security vocabulary for every repo in the isolate. The
+// record path now deep-copies (via clonePolicy) and the default is frozen, matching the non-record exit.
+test("#9998: normalizeCommandAuthorizationPolicy({}) returns fresh role arrays, deep-equal but not aliased", () => {
+  const policy = normalizeCommandAuthorizationPolicy({}).policy;
+  assert.notStrictEqual(policy.commands["review"], DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["review"]);
+  assert.deepEqual(policy.commands["review"], DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["review"]);
+  assert.deepEqual(policy.commands, DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands);
+  assert.deepEqual(policy.default, DEFAULT_COMMAND_AUTHORIZATION_POLICY.default);
+});
+
+test("#9998: the two exit paths agree — null and an override both return non-aliased arrays for un-overridden commands", () => {
+  const fromNull = normalizeCommandAuthorizationPolicy(null).policy;
+  assert.notStrictEqual(fromNull.commands["review"], DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["review"]);
+
+  // Overriding `plan` must not leave the un-overridden `pause` aliased to the default.
+  const fromOverride = normalizeCommandAuthorizationPolicy({ commands: { plan: ["maintainer"] } }).policy;
+  assert.notStrictEqual(fromOverride.commands["pause"], DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["pause"]);
+  assert.deepEqual(fromOverride.commands["pause"], DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["pause"]);
+});
+
+test("#9998: mutating a returned role array does not change the default, which is frozen", () => {
+  const review = normalizeCommandAuthorizationPolicy({}).policy.commands["review"];
+  assert.ok(review !== undefined);
+  review.push("pr_author");
+  assert.deepEqual(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["review"], ["maintainer", "collaborator", "confirmed_miner"]);
+  assert.equal(Object.isFrozen(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["generate-tests"]), true);
+});
+
+test("#9998: preserved behaviour — generate-tests stays maintainer-only and denies a COLLABORATOR", () => {
+  assert.deepEqual(commandAuthorizationAllowedRoles(null, "generate-tests"), ["maintainer"]);
+  assert.equal(
+    evaluateCommandAuthorization({ commandName: "generate-tests", commenterAssociation: "COLLABORATOR" }).authorized,
+    false,
+  );
+});

test/unit/command-authorization-engine.test.ts

@@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
 import {
   commandAuthorizationAllowedRoles,
   commandAuthorizationNeedsMinerDetection,
+  DEFAULT_COMMAND_AUTHORIZATION_POLICY,
   evaluateCommandAuthorization,
   normalizeCommandAuthorizationPolicy,
   summarizeCommandAuthorizationPolicy,
@@ -292,4 +293,21 @@ describe("repo command authorization policy", () => {
     expect(malformedCommands.warnings).toContain("commandAuthorization.commands must be an object; using command defaults.");
     expect(malformedCommands.policy.commands["queue-summary"]).toEqual(["maintainer", "collaborator"]);
   });
+
+  it("#9998: returns fresh, non-aliased role arrays on every input and freezes the default", () => {
+    // The record path used a shallow spread, sharing every un-overridden command's array with the module-level
+    // default; a caller mutating a returned array would corrupt the vocabulary for every repo in the isolate.
+    for (const input of [{}, null, { commands: { plan: ["maintainer"] } }] as const) {
+      const policy = normalizeCommandAuthorizationPolicy(input).policy;
+      // `pause` is never overridden by any of these inputs, so it exercises the deep-copied default path.
+      expect(policy.commands["pause"]).not.toBe(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["pause"]);
+      expect(policy.commands["pause"]).toEqual(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["pause"]);
+    }
+    const review = normalizeCommandAuthorizationPolicy({}).policy.commands["review"];
+    expect(review).toBeDefined();
+    review?.push("pr_author");
+    expect(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["review"]).toEqual(["maintainer", "collaborator", "confirmed_miner"]);
+    expect(Object.isFrozen(DEFAULT_COMMAND_AUTHORIZATION_POLICY.commands["generate-tests"])).toBe(true);
+    expect(Object.isFrozen(DEFAULT_COMMAND_AUTHORIZATION_POLICY)).toBe(true);
+  });
 });