diff --git a/docs/safety.md b/docs/safety.md index e98d5ecb..c8cb2dd6 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -7,18 +7,18 @@ Loops amplify judgment — good and bad. These guardrails are minimum bar for pr The loop must **never** auto-edit these without human approval: ``` -.env -.env.* +**/.env +**/.env.* **/secrets/** **/credentials/** **/*_key* **/*_secret* -.terraform/** -k8s/production/** +**/.terraform/** +**/k8s/production/** **/migrations/** # unless explicit migration loop -auth/** -payments/** -billing/** +**/auth/** +**/payments/** +**/billing/** ``` Encode in `minimal-fix` and implementer skills: diff --git a/gate.yaml b/gate.yaml index 60da1653..976f4b74 100644 --- a/gate.yaml +++ b/gate.yaml @@ -4,18 +4,18 @@ version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" - "**/*_secret*" - - ".terraform/**" - - "k8s/production/**" + - "**/.terraform/**" + - "**/k8s/production/**" - "**/migrations/**" - - "auth/**" - - "payments/**" - - "billing/**" + - "**/auth/**" + - "**/payments/**" + - "**/billing/**" maxFiles: 10 diff --git a/templates/gate.yaml.template b/templates/gate.yaml.template index 0eb433fa..2f348927 100644 --- a/templates/gate.yaml.template +++ b/templates/gate.yaml.template @@ -5,18 +5,18 @@ version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" - "**/*_secret*" - - ".terraform/**" - - "k8s/production/**" + - "**/.terraform/**" + - "**/k8s/production/**" - "**/migrations/**" - - "auth/**" - - "payments/**" - - "billing/**" + - "**/auth/**" + - "**/payments/**" + - "**/billing/**" # Escalate instead of auto-merging when a change touches more than this many files. maxFiles: 10 diff --git a/tools/loop-audit/dist/autofixer.js b/tools/loop-audit/dist/autofixer.js index 6cafa8dd..c0b4cb27 100644 --- a/tools/loop-audit/dist/autofixer.js +++ b/tools/loop-audit/dist/autofixer.js @@ -152,8 +152,8 @@ const GATE_YAML_TEMPLATE = `# Machine-readable twin of docs/safety.md, enforced version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" diff --git a/tools/loop-audit/src/autofixer.ts b/tools/loop-audit/src/autofixer.ts index 48f45e05..aba5662e 100644 --- a/tools/loop-audit/src/autofixer.ts +++ b/tools/loop-audit/src/autofixer.ts @@ -161,8 +161,8 @@ const GATE_YAML_TEMPLATE = `# Machine-readable twin of docs/safety.md, enforced version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" diff --git a/tools/loop-audit/test/autofixer.test.mjs b/tools/loop-audit/test/autofixer.test.mjs index 862f7b97..3111e211 100644 --- a/tools/loop-audit/test/autofixer.test.mjs +++ b/tools/loop-audit/test/autofixer.test.mjs @@ -46,6 +46,21 @@ test('autoFixProject writes a gate.yaml that loop-gate can actually load', async assert.match(written, /version:\s*1/); assert.match(written, /denylist:/); assert.doesNotMatch(written, /^gates:/m); + + // A bare pattern like ".env" only matches a path exactly equal to + // ".env" under minimatch -- it silently misses "services/api/.env". + // Every denylist entry here must be **/-anchored (or already contain + // its own ** segment) so it also catches the nested case, which is + // what "never auto-edit these" in docs/safety.md actually means. + const denylistBlock = written.match(/denylist:\n((?:\s+-\s+.+\n)+)/)?.[1] ?? ''; + const entries = [...denylistBlock.matchAll(/-\s+"([^"]+)"/g)].map((m) => m[1]); + assert.ok(entries.length > 0, 'expected at least one denylist entry to check'); + for (const entry of entries) { + assert.ok( + entry.startsWith('**/') || entry.includes('**'), + `denylist entry "${entry}" is not anchored and would miss a nested match`, + ); + } } finally { await rm(root, { recursive: true, force: true }).catch(() => {}); } diff --git a/tools/loop-gate/test/gate.test.mjs b/tools/loop-gate/test/gate.test.mjs index abb1103f..4a66d8ca 100644 --- a/tools/loop-gate/test/gate.test.mjs +++ b/tools/loop-gate/test/gate.test.mjs @@ -3,9 +3,13 @@ import assert from 'node:assert/strict'; import { mkdtemp, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { checkGate, loadGateConfig, assertValidAction } from '../dist/gate.js'; +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_GATE_YAML = path.resolve(__dirname, '../../../gate.yaml'); + const baseConfig = { version: 1, denylist: ['.env', '**/secrets/**', 'auth/**'], @@ -143,3 +147,26 @@ test('loadGateConfig rejects the wrong version', async () => { const file = await freshGateFile('version: 2\ndenylist: []\n'); await assert.rejects(() => loadGateConfig(file), /Invalid gate config/); }); + +// Dogfoods the actual policy this repo ships at gate.yaml, not just the +// matching mechanism -- a bare (unanchored) pattern like ".env" or "auth/**" +// only matches that exact root-level path and silently misses the same +// file/dir nested anywhere else, which a schema-validity check alone would +// never catch. +test("this repo's own gate.yaml denylist catches sensitive paths nested in a subdirectory, not just at repo root", async () => { + const config = await loadGateConfig(REPO_GATE_YAML); + const nestedSensitivePaths = [ + 'services/api/.env', + 'services/api/.env.production', + 'infra/.terraform/state.tfstate', + 'apps/backend/k8s/production/deploy.yaml', + 'apps/backend/auth/session.ts', + 'apps/checkout/payments/charge.ts', + 'apps/checkout/billing/invoice.ts', + ]; + for (const p of nestedSensitivePaths) { + const decision = checkGate({ config, action: 'commit', paths: [p] }); + assert.equal(decision.allowed, false, `${p} should be denylisted, got: ${decision.reason}`); + assert.equal(decision.trigger, 'denylist'); + } +}); diff --git a/tools/loop-sync/dist/sync.js b/tools/loop-sync/dist/sync.js index d9900dae..304255f8 100644 --- a/tools/loop-sync/dist/sync.js +++ b/tools/loop-sync/dist/sync.js @@ -162,18 +162,18 @@ Run log: (timestamp) | findings | actions | escalations version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" - "**/*_secret*" - - ".terraform/**" - - "k8s/production/**" + - "**/.terraform/**" + - "**/k8s/production/**" - "**/migrations/**" - - "auth/**" - - "payments/**" - - "billing/**" + - "**/auth/**" + - "**/payments/**" + - "**/billing/**" # Escalate instead of auto-merging when a change touches more than this many files. maxFiles: 10 diff --git a/tools/loop-sync/src/sync.ts b/tools/loop-sync/src/sync.ts index f0567f64..12238e55 100644 --- a/tools/loop-sync/src/sync.ts +++ b/tools/loop-sync/src/sync.ts @@ -210,18 +210,18 @@ Run log: (timestamp) | findings | actions | escalations version: 1 denylist: - - ".env" - - ".env.*" + - "**/.env" + - "**/.env.*" - "**/secrets/**" - "**/credentials/**" - "**/*_key*" - "**/*_secret*" - - ".terraform/**" - - "k8s/production/**" + - "**/.terraform/**" + - "**/k8s/production/**" - "**/migrations/**" - - "auth/**" - - "payments/**" - - "billing/**" + - "**/auth/**" + - "**/payments/**" + - "**/billing/**" # Escalate instead of auto-merging when a change touches more than this many files. maxFiles: 10 diff --git a/tools/loop-sync/test/sync.test.mjs b/tools/loop-sync/test/sync.test.mjs index b649b5a1..1e56cce0 100644 --- a/tools/loop-sync/test/sync.test.mjs +++ b/tools/loop-sync/test/sync.test.mjs @@ -102,6 +102,22 @@ describe('runSync auto-fix', () => { // loop-run-log.md must keep the marker append-run-log.mjs depends on. const runLog = await readFile(path.join(fixDir, 'loop-run-log.md'), 'utf8'); assert.match(runLog, //); + + // A bare pattern like ".env" only matches a path exactly equal to + // ".env" under minimatch (tools/loop-gate's matcher) -- it silently + // misses "services/api/.env". Every denylist entry must be + // **/-anchored (or already contain its own ** segment) so the + // scaffolded policy actually catches a nested match too. + const gateYaml = await readFile(path.join(fixDir, 'gate.yaml'), 'utf8'); + const denylistBlock = gateYaml.match(/denylist:\n((?:\s+-\s+.+\n)+)/)?.[1] ?? ''; + const entries = [...denylistBlock.matchAll(/-\s+"([^"]+)"/g)].map((m) => m[1]); + assert.ok(entries.length > 0, 'expected at least one denylist entry to check'); + for (const entry of entries) { + assert.ok( + entry.startsWith('**/') || entry.includes('**'), + `denylist entry "${entry}" is not anchored and would miss a nested match`, + ); + } }); test('does not fabricate LOOP.md or AGENTS.md -- still reported as missing', async () => {