From 8f45747069bf307d0cd584c7da93acab9eeed423 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Erbrech?= <stephane.erbrech@gmail.com>
Date: Wed, 29 Jul 2026 18:41:52 +1000
Subject: [PATCH] fix(security): exclude nested/alumni agent history paths from
 unsafe-git scanner
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The unsafe-git check in security-review.mjs scanned ALL changed files
with no file-type filter. When Scribe merged .squad/decisions/inbox/ into
decisions.md, the historical entry documenting the prohibition on git add .
became new diff lines — and the scanner flagged the very rule it was
documenting.

Root cause: .squad/decisions/inbox/ is gitignored, so inbox entries never
appeared in PR diffs. Once merged into decisions.md, those lines surface
as +lines in any PR that touches decisions.md.

Fix: add UNSAFE_GIT_EXCLUDED_PATHS to skip append-only memory/log files.

Original exclusion regex used `[^/]+` (exactly one path segment), which
failed to match nested alumni paths such as:
  .squad/agents/_alumni/kobayashi/history.md
The `_alumni/` subdirectory is the documented layout for retired agents
(see "Removing Team Members" flow), so this was a real false-positive, not
a hypothetical one. PR #1558 touches that exact path.

Corrected regex: `/^\.squad\/agents\/.+\/history\.md$/`
`.+` matches one or more characters including `/`, covering any nesting
depth while still anchoring on the literal filename `history.md`.

Excluded paths:
  - .squad/decisions.md            (Scribe-managed append-only decision log)
  - .squad/agents/**/history.md    (per-agent append-only memory, any depth)
  - .squad/log/**                  (gitignored runtime logs, listed for completeness)
  - .squad/orchestration-log/**    (gitignored, same)

Instruction surfaces intentionally kept in scope:
  - .github/copilot-instructions.md
  - .squad/agents/*/charter.md  (all depths — a malicious PR could plant unsafe
    git guidance in a charter; these MUST remain scannable)
  - .squad/team.md, .squad/routing.md, squad.agent.md

Verified locally:
  (a) Exclusions work:
        .squad/decisions.md                          → excluded
        .squad/agents/retro/history.md               → excluded
        .squad/agents/_alumni/kobayashi/history.md   → excluded (was broken)
        .squad/agents/_alumni/deep/nested/history.md → excluded (any depth)
  (b) Charters still scanned:
        .squad/agents/retro/charter.md    → NOT excluded (scanned)
        .squad/agents/scribe/charter.md   → NOT excluded (scanned)
  (c) Probe detection:
        scripts/__scan-probe.sh with `git add .` and `git push --force`
        → 2 errors detected; probe deleted, no trace in commit.
  (d) PR #1558 (squad/state-sync-2026-07-29):
        node scripts/security-review.mjs origin/dev FETCH_HEAD
        → no findings (alumni history correctly excluded)

Known follow-up (not fixed here): /git\s+add\s+\./ also matches the safe
scoped form `git add .squad/`. Separate pattern-precision issue documented
in PR body.

Refs #1556

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 scripts/security-review.mjs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/scripts/security-review.mjs b/scripts/security-review.mjs
index eef502ed8..06d0db7f5 100644
--- a/scripts/security-review.mjs
+++ b/scripts/security-review.mjs
@@ -174,6 +174,21 @@ for (const file of jstsFiles) {
 }
 
 // 4. Unsafe git operations
+//
+// Excluded paths: append-only memory/log files that document history and prohibitions.
+// These are never executed — they record what happened, not what to do.
+// Instruction surfaces (.github/copilot-instructions.md, agent charters, squad.agent.md,
+// team.md, routing.md) are deliberately kept in scope: a malicious PR could replace a
+// prohibition with an instruction, and we want the scanner to catch that.
+// .squad/log/ and .squad/orchestration-log/ are gitignored (never in PR diffs) but
+// listed here for documentation completeness.
+const UNSAFE_GIT_EXCLUDED_PATHS = [
+  /^\.squad\/decisions\.md$/,
+  /^\.squad\/agents\/.+\/history\.md$/,
+  /^\.squad\/log\//,
+  /^\.squad\/orchestration-log\//,
+];
+
 const GIT_UNSAFE_PATTERNS = [
   { pattern: /git\s+add\s+\./, label: 'git add .' },
   { pattern: /git\s+add\s+-A/, label: 'git add -A' },
@@ -183,6 +198,7 @@ const GIT_UNSAFE_PATTERNS = [
 ];
 
 for (const file of changedFiles) {
+  if (UNSAFE_GIT_EXCLUDED_PATHS.some((p) => p.test(file))) continue;
   const added = addedByFile.get(file) || [];
   for (const { line, text } of added) {
     for (const { pattern, label } of GIT_UNSAFE_PATTERNS) {