Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions .codearbiter/security-controls.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,31 @@ Hook input parsing fails open (not closed) on malformed stdin — see

## Audit trail

`overrides.log` and `triage.log` are append-only artifacts. They may never be
truncated, rewritten, or deleted. The `pre-bash.py` H-05 guard and the
`pre-write.py` / `pre-edit.py` H-05 guards enforce this at every tool-call
boundary.
`overrides.log`, `triage.log`, and `sprint-log.md` are append-only artifacts.
They may never be truncated, rewritten, or deleted. The `pre-bash.py` H-05 guard
and the `pre-write.py` / `pre-edit.py` H-05 guards enforce this at every
tool-call boundary.

**Enforcement scope (accepted residual risk).** These guards are *integrity*
controls, not *completeness* controls — they protect a log once written, they do
not compel a write (see `observability-002` / the "compel a log write" CONFIRM).
The `pre-bash.py` shell guard is lexical and anchored on the literal log name, so
the following truncation/indirection spellings are out of scope and accepted as
residual risk (the sanctioned bypass for legitimate log management is
`/ca:override`):

- file-descriptor redirects where no filename token is adjacent to the verb —
`exec 3>.codearbiter/overrides.log`;
- triple-chevron `>>>` (treated as append by some shells);
- process-substitution spellings;
- verb-with-variable targets where the literal name never appears beside the
verb — `f=.codearbiter/overrides.log; rm "$f"` (bash) or `$f='overrides.log';
rm $f` (PowerShell).

The `pre-write.py` / `pre-edit.py` guards close the Write/Edit flank (including an
empty-`old_string` Edit, which is not a verifiable append). The append-only path
set is centralized in `_hooklib` (`is_audit_log`, `AUDIT_LOG_NAMES`) so the three
guards never drift on which files are covered.

---

Expand Down
95 changes: 95 additions & 0 deletions .github/scripts/test_hooklib.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Stdlib only. Exit 0 = all tests pass; non-zero = failure.
"""

import json
import os
import sys
import tempfile
Expand All @@ -18,6 +19,7 @@
HERE = os.path.dirname(os.path.abspath(__file__))
REPO = os.path.dirname(os.path.dirname(HERE))
HOOKS = os.path.join(REPO, "plugins", "ca", "hooks")
SECRET_CORPUS = os.path.join(HOOKS, "secret-detection-corpus.json")
sys.path.insert(0, HOOKS)

import _hooklib # noqa: E402 — needs sys.path mutation above
Expand Down Expand Up @@ -55,6 +57,18 @@ def test_still_matches_banned_primitives(self):
for s in ("createHash('md5')", "import bcrypt", "new DES()", "x509 cert"):
self.assertTrue(self._matches(s), s)

# --- secrets-001: RC2 and Blowfish are forbidden by security-controls.md
# but were absent from CRYPTO_RE, so a commit adding either passed the
# H-09b gate with no crypto-compliance review. ---
def test_matches_rc2(self):
for s in ("createCipheriv('rc2-cbc', key, iv)", "new RC2(key)"):
self.assertTrue(self._matches(s), s)

def test_matches_blowfish(self):
for s in ("new Blowfish(key)", "cipher = blowfish.new(key)",
"algorithm: BLOWFISH"):
self.assertTrue(self._matches(s), s)

# --- direct coverage for every CRYPTO_RE branch (checkpoint 2026-06-22
# NEEDS-TRIAGE: ~18 branches were exercised only indirectly). Each string
# is chosen to match via its named branch only (no incidental RSA/sha1). ---
Expand Down Expand Up @@ -116,6 +130,16 @@ def test_matches_ts_object_key(self):
def test_matches_aws_secret_keyword(self):
self.assertTrue(self._matches('aws_secret_access_key = "wJalrXUtnFEMI1234"'))

# --- secrets-002: compound names like FARM_API_KEY. The leading `\b` on the
# keyword alternation did NOT fire before `api_key` when the char to its
# left is a word char (the `_` in FARM_API_KEY), so a hardcoded
# `FARM_API_KEY = "..."` silently passed the H-10b secret gate. ---
def test_matches_compound_name_underscore_prefix(self):
self.assertTrue(self._matches('const FARM_API_KEY = "sk-CVlQvxKsecretvalue123";'))

def test_matches_compound_name_object_form(self):
self.assertTrue(self._matches('MY_SECRET: "longsecretvalue123"'))

# --- known high-entropy key prefixes (keyword-independent) ---
def test_matches_aws_access_key_id_prefix(self):
self.assertTrue(self._matches("AKIAIOSFODNN7EXAMPLE"))
Expand All @@ -134,6 +158,77 @@ def test_does_not_match_prose_mention(self):
self.assertFalse(self._matches("# load the secret from the env store"))


class SecretCorpusTest(unittest.TestCase):
"""architecture-001: the SECRET_RE commit gate and farm.ts's SECRET_LINE
outbound redactor are deliberately distinct in shape, but must never drift
apart on the AGREEMENT region — real secret shapes both must flag, and benign
lines both must pass. This pins the Python (SECRET_RE) side of that shared
corpus; plugins/ca/tools/farm.unit.test.ts pins the TS (SECRET_LINE) side
against the SAME file, so a divergence on any entry fails CI on one side."""

@classmethod
def setUpClass(cls):
with open(SECRET_CORPUS, encoding="utf-8") as f:
cls.corpus = json.load(f)

def _matches(self, s):
return bool(_hooklib.SECRET_RE.search(s))

def test_corpus_has_both_sets(self):
self.assertTrue(self.corpus.get("must_match"), "corpus must_match is empty")
self.assertTrue(self.corpus.get("must_not_match"), "corpus must_not_match is empty")

def test_secret_re_flags_every_must_match(self):
for s in self.corpus["must_match"]:
with self.subTest(line=s):
self.assertTrue(self._matches(s),
"SECRET_RE must flag corpus secret: %r" % s)

def test_secret_re_passes_every_must_not_match(self):
for s in self.corpus["must_not_match"]:
with self.subTest(line=s):
self.assertFalse(self._matches(s),
"SECRET_RE must NOT flag benign corpus line: %r" % s)


class AuditPathHelperTest(unittest.TestCase):
"""architecture-004: the append-only-log and ADR-decisions path sets were
triplicated inline across pre-write/pre-edit/pre-bash. They now live once in
_hooklib (the same home as CRYPTO_RE/SECRET_RE/MIGRATION globs) so adding an
audit artifact touches one file. These pin the centralized API + its scope."""

def test_is_audit_log_matches_the_three_append_only_files(self):
for rel in (".codearbiter/overrides.log",
".codearbiter/triage.log",
".codearbiter/sprint-log.md"):
with self.subTest(rel=rel):
self.assertTrue(_hooklib.is_audit_log(rel), rel)

def test_is_audit_log_normalizes_windows_separators(self):
self.assertTrue(_hooklib.is_audit_log(".codearbiter\\overrides.log"))

def test_is_audit_log_rejects_non_audit_paths(self):
for rel in (".codearbiter/CONTEXT.md", "src/overrides.log.bak",
".codearbiter/open-tasks.md"):
with self.subTest(rel=rel):
self.assertFalse(_hooklib.is_audit_log(rel), rel)

def test_is_decisions_path_matches_adrs(self):
for rel in (".codearbiter/decisions/0001-seed.md",
".codearbiter/decisions/draft.md",
".codearbiter/decisions/sub/0002-x.md"):
with self.subTest(rel=rel):
self.assertTrue(_hooklib.is_decisions_path(rel), rel)

def test_is_decisions_path_normalizes_windows_separators(self):
self.assertTrue(_hooklib.is_decisions_path(".codearbiter\\decisions\\0001-x.md"))

def test_is_decisions_path_rejects_non_decisions(self):
for rel in (".codearbiter/CONTEXT.md", "src/decisions/x.md"):
with self.subTest(rel=rel):
self.assertFalse(_hooklib.is_decisions_path(rel), rel)


class PathGlobTest(unittest.TestCase):
"""is_migration_path / is_ci_path / is_deploy_path against default globs
(no security-controls.md in the tmp root, so defaults only)."""
Expand Down
41 changes: 40 additions & 1 deletion plugins/ca/hooks/_hooklib.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
# is_migration_path(rel, root) -> bool True iff rel is a DB migration (H-14)
# is_ci_path(rel, root) -> bool True iff rel is a CI/CD workflow (H-15)
# is_deploy_path(rel, root) -> bool True iff rel is a deployment/IaC manifest (H-16)
# is_audit_log(rel) -> bool True iff rel is an append-only audit log (H-05)
# is_decisions_path(rel) -> bool True iff rel is an ADR under decisions/ (H-11)
# marker_fresh(path, minutes) -> bool True iff marker file exists and is recent
# block(tag, msg) -> None BLOCK tool call: print to stderr and exit 2
# remind(tag, msg) -> None non-blocking nudge to stderr
Expand All @@ -62,6 +64,7 @@
# not, a password-hashing change is exactly what crypto-compliance reviews.
CRYPTO_RE = re.compile(
r"(createHash|createCipher|createHmac|\bmd5\b|\bsha1\b|\brc4\b|\bdes\b|3des"
r"|\brc2\b|\bblowfish\b"
r"|\bRSA\b|x509|bcrypt"
r"|crypto\.(subtle|sign|verify|createSign|createVerify|generateKey"
r"|publicEncrypt|privateDecrypt|pbkdf2|scrypt|randomBytes|createDiffieHellman)"
Expand All @@ -75,8 +78,14 @@
# (the colon/object form dominates this TS/JSON repo) — the quoted-value
# requirement keeps it from firing on every bare `token:` reference; (2) known
# high-entropy key prefixes, keyword-independent (AWS / GitHub / Anthropic).
# No LEADING word boundary on the keyword group (secrets-002): a `\b` there
# never fires when the keyword is the trailing segment of a compound identifier
# (the char left of `api_key` in `FARM_API_KEY` is `_`, a word char), so a
# hardcoded `FARM_API_KEY = "..."` silently passed the gate. The right-hand
# quoted-assignment anchor still bounds the match — a bare `token:` reference
# without a quoted value never matches.
SECRET_RE = re.compile(
r"\b(?:password|secret|token|api_key|apikey|private_key|passphrase|credential"
r"(?:password|secret|token|api_key|apikey|private_key|passphrase|credential"
r"|aws_secret_access_key|client_secret)"
r"""["']?\s*[:=]\s*["'][^"']{4,}"""
r"|AKIA[0-9A-Z]{16}"
Expand All @@ -88,6 +97,36 @@

ARBITER_RE = re.compile(r"^\s*arbiter:\s*enabled\s*$", re.I)

# Append-only audit logs (H-05) and ADR-decisions paths (H-11) — centralized
# here (architecture-004) so the three pre-* hooks import ONE definition instead
# of re-encoding the regex inline (the exact drift this module exists to
# prevent: adding sprint-log.md once meant hand-editing every copy). Same home,
# same rationale, as CRYPTO_RE/SECRET_RE/MIGRATION_DEFAULT_GLOBS.
#
# AUDIT_LOG_NAMES is the bare filename alternation; pre-bash.py composes its
# shell LOG_NAMES from it, and AUDIT_LOG_RE anchors it under .codearbiter/ for
# the Write/Edit file-path guards. DECISIONS_DIR_RE is the separator-tolerant
# decisions directory token; pre-bash.py composes its shell DECISIONS from it,
# and DECISIONS_PATH_RE extends it to a full ADR file path. `[\\/]+` matches the
# norm_path'd `/` as well as a raw backslash, so both the file-path and shell
# flanks derive from one source.
AUDIT_LOG_NAMES = r"(?:(?:overrides|triage)\.log|sprint-log\.md)"
AUDIT_LOG_RE = re.compile(r"\.codearbiter/" + AUDIT_LOG_NAMES + r"$")
DECISIONS_DIR_RE = r"\.codearbiter[\\/]+decisions"
DECISIONS_PATH_RE = re.compile(DECISIONS_DIR_RE + r"[\\/]+.+\.md$")


def is_audit_log(rel):
"""True iff `rel` is one of the append-only .codearbiter audit logs
(overrides.log, triage.log, sprint-log.md) — the H-05 guard set."""
return bool(AUDIT_LOG_RE.search(norm_path(rel)))


def is_decisions_path(rel):
"""True iff `rel` is a `.md` ADR anywhere under .codearbiter/decisions/ —
the H-11 guard set (a non-numbered draft or a nested path still counts)."""
return bool(DECISIONS_PATH_RE.search(norm_path(rel)))


def utf8_stdio():
"""Force UTF-8 on stdout/stderr. Windows pipes default to the locale code
Expand Down
24 changes: 15 additions & 9 deletions plugins/ca/hooks/pre-bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@

sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
from _hooklib import ( # noqa: E402
CRYPTO_RE, SECRET_RE, arbiter_active, block, content_digest, is_migration_path,
line_digest, marker_fresh, project_root, read_input, tool_input, utf8_stdio,
AUDIT_LOG_NAMES, CRYPTO_RE, DECISIONS_DIR_RE, SECRET_RE, arbiter_active, block,
content_digest, is_migration_path, line_digest, marker_fresh, project_root,
read_input, tool_input, utf8_stdio,
)

# `git` followed by any run of global options (-C <dir>, -c k=v, --git-dir=…,
Expand Down Expand Up @@ -61,15 +62,20 @@
# N-3: Known limitations — this regex catches the common `> file` and `>| file`
# (force-clobber) truncation forms but NOT every shell spelling that produces a
# new file descriptor on the log. Specific gaps: triple-chevron (`>>>`, treated
# as append by some shells), and file-descriptor forms like
# `exec 3>.codearbiter/overrides.log`. These are difficult to close with a single
# regex and represent an accepted residual risk. The sanctioned bypass for
# legitimate log management is /ca:override.
# as append by some shells), file-descriptor forms like
# `exec 3>.codearbiter/overrides.log`, and verb-with-VARIABLE-target spellings
# where the literal log name never appears adjacent to the verb (appsec-003):
# e.g. `f=.codearbiter/overrides.log; rm "$f"` or PowerShell `$f='overrides.log';
# rm $f` — the guard is purely lexical and anchored on the literal name, so an
# indirected target defeats it. These are difficult to close with a single regex
# and represent an accepted residual risk. The sanctioned bypass for legitimate
# log management is /ca:override.
# The optional `\|?` admits `>|` (clobber even under `set -o noclobber`); the
# leading `(?!>)` still excludes the append form `>>`.
# `sprint-log.md` joins overrides.log/triage.log as an append-only audit artifact
# (the /sprint decision record).
LOG_NAMES = r"(?:(?:overrides|triage)\.log|sprint-log\.md)"
# (the /sprint decision record). The bare-name alternation is centralized in
# _hooklib.AUDIT_LOG_NAMES so the Write/Edit and shell flanks never drift.
LOG_NAMES = AUDIT_LOG_NAMES
LOG_TRUNC_RE = re.compile(r"(?<!>)>(?!>)\|?\s*\S*" + LOG_NAMES)
LOG_DESTROY_RE = re.compile(
r"\b(rm|del|mv|cp|copy|dd|tee|sed|truncate|sponge"
Expand All @@ -80,7 +86,7 @@
# guard the Write/Edit tools; this guards redirection and file verbs). Any
# redirect into .codearbiter/decisions/, or any write/delete verb naming it,
# blocks — `cat`/`ls`/`grep` reads pass untouched.
DECISIONS = r"\.codearbiter[\\/]+decisions\b"
DECISIONS = DECISIONS_DIR_RE + r"\b"
# `>>?\|?` covers `>`, `>>`, and the `>|` force-clobber form into decisions/.
DECISIONS_REDIRECT_RE = re.compile(r">>?\|?\s*\S*" + DECISIONS, re.I)
DECISIONS_WRITE_RE = re.compile(
Expand Down
23 changes: 17 additions & 6 deletions plugins/ca/hooks/pre-edit.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@
# backslash paths too. Runs only in arbiter-enabled repos.

import os
import re
import sys

sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
from _hooklib import ( # noqa: E402
arbiter_active, block, marker_fresh, norm_path, project_root, read_input,
tool_input, utf8_stdio,
arbiter_active, block, is_audit_log, is_decisions_path, marker_fresh,
norm_path, project_root, read_input, tool_input, utf8_stdio,
)


Expand All @@ -31,8 +30,8 @@ def main():

# H-05: the audit logs (and the /sprint decision record) are append-only — an
# Edit must leave existing lines intact (new_string extends old_string), never
# alter or delete them.
if re.search(r"\.codearbiter/(?:(?:overrides|triage)\.log|sprint-log\.md)$", fpath):
# alter or delete them. (path set: _hooklib.is_audit_log)
if is_audit_log(fpath):
# MultiEdit applies a batch of edits and cannot express a verifiable pure
# append to an append-only file — the sanctioned append path is a single
# Edit (or '>>'). Block it outright rather than reason about the batch.
Expand All @@ -42,6 +41,17 @@ def main():
"(ORCHESTRATOR §7). Append with a single Edit or '>>'.")
old = ti.get("old_string", "") or ""
new = ti.get("new_string", "") or ""
# migration-003: `new.startswith("")` is ALWAYS True, so an empty
# old_string defeats the append check entirely — it could prepend or
# replace arbitrary content with the guard never firing. An empty
# old_string can never be a verifiable append (there is no existing tail
# to extend); block it outright.
if not old:
block("H-05", "An Edit with an empty old_string on an append-only .codearbiter "
"audit log (overrides.log, triage.log, sprint-log.md) cannot be a "
"verifiable pure append (ORCHESTRATOR §7) — every string starts with "
"the empty string. Append with '>>', or a single Edit whose old_string "
"is the file's current trailing content.")
if not new.startswith(old):
block("H-05", "The .codearbiter audit logs (overrides.log, triage.log, "
"sprint-log.md) are append-only (ORCHESTRATOR §7). This Edit alters "
Expand All @@ -50,7 +60,8 @@ def main():

# H-11: ADRs may only be edited via /adr — any .md anywhere under decisions/
# (a non-numbered draft or a nested path is still an immutable decision).
if re.search(r"\.codearbiter/decisions/.+\.md$", fpath):
# (path set: _hooklib.is_decisions_path)
if is_decisions_path(fpath):
marker = os.path.join(root, ".codearbiter", ".markers", "adr-authoring-active")
if not os.path.isfile(marker):
block("H-11", "ADR files are edited only via /adr (ORCHESTRATOR §3) — user "
Expand Down
13 changes: 6 additions & 7 deletions plugins/ca/hooks/pre-write.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
# repos.

import os
import re
import sys

sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
from _hooklib import ( # noqa: E402
arbiter_active, block, marker_fresh, norm_path, project_root, read_input,
tool_input, utf8_stdio,
arbiter_active, block, is_audit_log, is_decisions_path, marker_fresh,
norm_path, project_root, read_input, tool_input, utf8_stdio,
)


Expand All @@ -26,15 +25,15 @@ def main():
fpath = norm_path(tool_input(read_input()).get("file_path", "") or "")

# H-05: the audit logs (and the /sprint decision record) are append-only —
# a Write is a full overwrite.
if re.search(r"\.codearbiter/(?:(?:overrides|triage)\.log|sprint-log\.md)$", fpath):
# a Write is a full overwrite. (path set: _hooklib.is_audit_log)
if is_audit_log(fpath):
block("H-05", "The .codearbiter audit logs (overrides.log, triage.log, sprint-log.md) "
"are append-only (ORCHESTRATOR §7). Append with Edit or '>>', never Write.")

# H-11: ADRs may only be authored via /adr (the skill drops the marker first).
# Any .md anywhere under decisions/ is covered — a non-numbered draft or a
# nested path is still an immutable decision artifact.
if re.search(r"\.codearbiter/decisions/.+\.md$", fpath):
# nested path is still an immutable decision artifact. (set: is_decisions_path)
if is_decisions_path(fpath):
marker = os.path.join(root, ".codearbiter", ".markers", "adr-authoring-active")
if not os.path.isfile(marker):
block("H-11", "ADR files are authored only via /adr (ORCHESTRATOR §3) — user "
Expand Down
Loading
Loading