mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
fix(security_guards): reject un-allowlisted .gitignore negations (#195)
check_gitignore() verified each required personal-data rule was present via set membership, but .gitignore is order-sensitive: a later !pattern re-includes a file an earlier rule excluded, so the required line stays physically present while the file is no longer ignored - the guard failed open on exactly the weakening its docstring claims to catch. Keeps the required-rules-present check and additionally rejects any negation line outside a small reviewed ALLOWED_IGNORE_NEGATIONS allowlist (same explicit-widening pattern as ALLOWED_PERMISSIONS). Fixes #194. By @thejesh23. Verified: allowlist matches the four negations currently in .gitignore; guard test suite passes locally (17 tests) and in CI. Closes #194
This commit is contained in:
@@ -125,6 +125,29 @@ class GitignoreGuardTests(GuardRepoFixture):
|
|||||||
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
|
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
|
||||||
|
|
||||||
|
|
||||||
|
class GitignoreNegationTests(GuardRepoFixture):
|
||||||
|
def test_negation_reincluding_personal_data_fails(self):
|
||||||
|
# .gitignore is order-sensitive: `!salary_data.json` after the
|
||||||
|
# `salary_data.json` rule re-includes the file, so the required rule is
|
||||||
|
# still present but no longer takes effect. Set membership on the
|
||||||
|
# required rules cannot see this, so the negation must be rejected.
|
||||||
|
self.write_gitignore(list(security_guards.REQUIRED_IGNORE_RULES) + ["!salary_data.json"])
|
||||||
|
result = run_guards(self.root)
|
||||||
|
self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
|
||||||
|
self.assertIn("negation rule not in the reviewed allowlist", result.stdout)
|
||||||
|
self.assertIn("!salary_data.json", result.stdout)
|
||||||
|
|
||||||
|
def test_allowlisted_negations_pass(self):
|
||||||
|
# The template's own benign negations (example CV/cover letter, fonts,
|
||||||
|
# .gitkeep placeholders) must keep passing.
|
||||||
|
self.write_gitignore(
|
||||||
|
list(security_guards.REQUIRED_IGNORE_RULES)
|
||||||
|
+ sorted(security_guards.ALLOWED_IGNORE_NEGATIONS)
|
||||||
|
)
|
||||||
|
result = run_guards(self.root)
|
||||||
|
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
|
||||||
|
|
||||||
|
|
||||||
class ManifestGuardTests(GuardRepoFixture):
|
class ManifestGuardTests(GuardRepoFixture):
|
||||||
def test_each_lifecycle_script_fails(self):
|
def test_each_lifecycle_script_fails(self):
|
||||||
for script in sorted(security_guards.FORBIDDEN_SCRIPTS):
|
for script in sorted(security_guards.FORBIDDEN_SCRIPTS):
|
||||||
|
|||||||
@@ -13,9 +13,10 @@ Checks:
|
|||||||
1. .claude/settings.json — every permissions.allow entry must be in the exact
|
1. .claude/settings.json — every permissions.allow entry must be in the exact
|
||||||
allowlist below. Catches permission widening (e.g. Bash(*), Bash(curl:*)),
|
allowlist below. Catches permission widening (e.g. Bash(*), Bash(curl:*)),
|
||||||
which would auto-approve commands on every fork.
|
which would auto-approve commands on every fork.
|
||||||
2. .gitignore — the personal-data ignore rules must all still be present.
|
2. .gitignore — the personal-data ignore rules must all still be present,
|
||||||
Catches weakening that would make future users silently commit their
|
and no un-allowlisted negation (!pattern) may re-include them. Catches
|
||||||
tracker, profile exports, or application archives.
|
weakening that would make future users silently commit their tracker,
|
||||||
|
profile exports, or application archives.
|
||||||
3. .agents/**/package.json — no npm/bun lifecycle scripts (preinstall,
|
3. .agents/**/package.json — no npm/bun lifecycle scripts (preinstall,
|
||||||
install, postinstall, prepare, prepack) and no trustedDependencies.
|
install, postinstall, prepare, prepack) and no trustedDependencies.
|
||||||
Catches code execution smuggled into `bun install`.
|
Catches code execution smuggled into `bun install`.
|
||||||
@@ -55,6 +56,20 @@ REQUIRED_IGNORE_RULES = [
|
|||||||
"job_search_tracker.csv",
|
"job_search_tracker.csv",
|
||||||
]
|
]
|
||||||
|
|
||||||
|
# Negation (re-include) rules the template legitimately ships. .gitignore is
|
||||||
|
# order-sensitive: a later `!pattern` re-includes a path an earlier rule
|
||||||
|
# excluded, so a rule can be physically present in REQUIRED_IGNORE_RULES yet
|
||||||
|
# no longer ignored (e.g. adding `!salary_data.json`). Set membership on the
|
||||||
|
# required rules cannot see that. Any negation outside this allowlist is a
|
||||||
|
# failure - add an intentional one here in the same PR, exactly as with
|
||||||
|
# ALLOWED_PERMISSIONS, so the widening is explicit and reviewable.
|
||||||
|
ALLOWED_IGNORE_NEGATIONS = {
|
||||||
|
"!cover_letters/OpenFonts/fonts/**",
|
||||||
|
"!cv/main_example.tex",
|
||||||
|
"!cover_letters/cover_example.tex",
|
||||||
|
"!documents/**/.gitkeep",
|
||||||
|
}
|
||||||
|
|
||||||
FORBIDDEN_SCRIPTS = {"preinstall", "install", "postinstall", "prepare", "prepack"}
|
FORBIDDEN_SCRIPTS = {"preinstall", "install", "postinstall", "prepare", "prepack"}
|
||||||
|
|
||||||
|
|
||||||
@@ -93,10 +108,11 @@ def check_permissions() -> None:
|
|||||||
def check_gitignore() -> None:
|
def check_gitignore() -> None:
|
||||||
path = ROOT / ".gitignore"
|
path = ROOT / ".gitignore"
|
||||||
try:
|
try:
|
||||||
rules = {line.strip() for line in path.read_text(encoding="utf-8").splitlines()}
|
lines = [line.strip() for line in path.read_text(encoding="utf-8").splitlines()]
|
||||||
except OSError as exc:
|
except OSError as exc:
|
||||||
errors.append(f".gitignore: unreadable: {exc}")
|
errors.append(f".gitignore: unreadable: {exc}")
|
||||||
return
|
return
|
||||||
|
rules = set(lines)
|
||||||
for rule in REQUIRED_IGNORE_RULES:
|
for rule in REQUIRED_IGNORE_RULES:
|
||||||
if rule not in rules:
|
if rule not in rules:
|
||||||
errors.append(
|
errors.append(
|
||||||
@@ -105,6 +121,15 @@ def check_gitignore() -> None:
|
|||||||
"or was renamed intentionally, update REQUIRED_IGNORE_RULES in "
|
"or was renamed intentionally, update REQUIRED_IGNORE_RULES in "
|
||||||
"tools/security_guards.py in the same PR."
|
"tools/security_guards.py in the same PR."
|
||||||
)
|
)
|
||||||
|
for line in lines:
|
||||||
|
if line.startswith("!") and line not in ALLOWED_IGNORE_NEGATIONS:
|
||||||
|
errors.append(
|
||||||
|
f".gitignore: negation rule not in the reviewed allowlist: {line!r}. "
|
||||||
|
"A negation re-includes a path an earlier rule excluded and can silently "
|
||||||
|
"re-expose personal data (a required ignore rule stays present but stops "
|
||||||
|
"taking effect). If this negation is intentional, add it to "
|
||||||
|
"ALLOWED_IGNORE_NEGATIONS in tools/security_guards.py in the same PR."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def check_package_manifests() -> None:
|
def check_package_manifests() -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user