mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
fix(security): move the interview protection note to the rule that provides it (#337)
The two-line comment above `documents/interview/**` says interview prep and experience records live there. Nothing has ever written to that directory: /interview saves its pack to documents/applications/<company>_<role>/interview_prep_<stage>.md, covered by the documents/applications/** rule. `git grep documents/interview` returns only the two declarations of the rule itself (.gitignore and REQUIRED_IGNORE_RULES), `git log --all -- 'documents/interview*'` is empty, and documents/README.md documents the applications path outright. Nothing leaks - the comment is the defect, and it is the misleading kind. It is the one dedicated, well-argued line about interview material in the personal-data block, so an auditor checking that the framework's most sensitive artifact is covered reads it and stops, at the only path in the block with no writer. The comment's description of what needs protecting was always right; only its location was wrong. It now sits above documents/applications/**, the rule that actually provides that protection, so a reader auditing the block finds the reasoning attached to the rule doing the work. documents/interview/** stays - REQUIRED_IGNORE_RULES pins it, so dropping it from .gitignore alone turns CI red, and it is harmless defence in depth - relabelled in both files as belt-and-braces rather than the primary guard. The new check-ignore case in GitignorePatternBehaviorTests derives the prep-pack path from /interview's own spec instead of hardcoding it. That distinction is the whole value of the test: a hardcoded path pins only that documents/applications/** still matches that shape, which security_guards.py already catches first, and stays green if /interview moves its output - leaving the corrected comment stale exactly the way this issue found it. Since #329 the spec states the location in two pieces - Step 1 derives the archive folder, Step 3 names interview_prep_<stage>.md - so the test pins both fragments separately and composes the concrete path from them. Mutation- verified on each half: repointing the folder at documents/prep_packs/, and renaming the file, both fail this test while `python3 tools/security_guards.py` still reports OK. The class's temp-repo setup moved to setUp for the second case. ayobamiseun reviewed the pre-rebase branch and called all three rebase hazards in advance: the split literal, the released CHANGELOG context, and the setUp re-merge. Reached independently here during the rebase; the review was posted first.
This commit is contained in:
+6
-2
@@ -73,10 +73,14 @@ documents/cv/**
|
||||
documents/linkedin/**
|
||||
documents/diplomas/**
|
||||
documents/references/**
|
||||
# Also where /interview saves its prep packs (interview_prep_<stage>.md): these
|
||||
# name the employers applied to, quote what was submitted, and set out the
|
||||
# candidate's weak points.
|
||||
documents/applications/**
|
||||
documents/postings/**
|
||||
# Interview prep and experience records: these name the employers applied to,
|
||||
# quote what was submitted, and set out the candidate's weak points.
|
||||
# Belt-and-braces, not the primary guard: nothing writes here. Prep packs land
|
||||
# in documents/applications/<company>_<role>/, covered above. Kept because
|
||||
# tools/security_guards.py pins it in REQUIRED_IGNORE_RULES.
|
||||
documents/interview/**
|
||||
!documents/**/.gitkeep
|
||||
|
||||
|
||||
@@ -26,6 +26,20 @@ per-file diff commands.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The `documents/interview/**` ignore rule no longer claims interview prep is written there**
|
||||
(#336). `/interview` saves its pack to
|
||||
`documents/applications/<company>_<role>/interview_prep_<stage>.md`, already ignored by
|
||||
`documents/applications/**`; nothing has ever written to `documents/interview/`. Nothing leaked -
|
||||
but it was the personal-data block's one dedicated line about interview material, so an auditor
|
||||
checking the framework's most sensitive artifact had every reason to read it and stop, at the
|
||||
only path in the block with no writer. The protection rationale now sits above
|
||||
`documents/applications/**`, the rule that actually provides it, so the next reader finds it
|
||||
where it lives; `documents/interview/**` stays, relabelled belt-and-braces rather than primary
|
||||
guard (`REQUIRED_IGNORE_RULES` pins it, so removing it from `.gitignore` alone fails the guard).
|
||||
Pinned by `tests/test_security_guards.py`, which derives the prep-pack path from
|
||||
`/interview`'s own spec instead of hardcoding it - so moving that path fails CI rather than
|
||||
quietly re-staling the comment.
|
||||
|
||||
- **`/scrape` now persists each posting's publication date** (#390) - Step 2's contract guarantees a
|
||||
`date` on every portal CLI's search output (CI enforces it in `test_scrape_contract.py`) and
|
||||
Step 1b uses that date to scope a run to the last 14 days, but Step 4's `seen_jobs.json` schema
|
||||
|
||||
@@ -261,24 +261,28 @@ class GitignoreGuardTests(GuardRepoFixture):
|
||||
|
||||
|
||||
class GitignorePatternBehaviorTests(unittest.TestCase):
|
||||
"""Pin the match semantics of the shipped .gitignore for upskill reports.
|
||||
"""Pin the match semantics of the shipped .gitignore, not just rule presence.
|
||||
|
||||
The upskill skill resolves `upskill/` relative to its own directory (the
|
||||
same observed behavior the **/job_scraper rules exist for), so a report
|
||||
must be ignored at that depth too. The skill's own SKILL.md lives in a
|
||||
directory that shares the `upskill` name, so a broad `**/upskill/*.md`
|
||||
would ignore the template's own skill file - this pins that it stays
|
||||
tracked. Guard presence checks cannot see either property; only real
|
||||
check-ignore semantics can.
|
||||
The guard checks that a rule exists; it never checks what the rule matches.
|
||||
These cases run real `git check-ignore` over the shipped file, for paths the
|
||||
framework actually writes.
|
||||
"""
|
||||
|
||||
def test_upskill_reports_ignored_at_depth_but_skill_md_stays_tracked(self):
|
||||
root = Path(tempfile.mkdtemp())
|
||||
self.addCleanup(shutil.rmtree, root, ignore_errors=True)
|
||||
def setUp(self):
|
||||
self.root = Path(tempfile.mkdtemp())
|
||||
self.addCleanup(shutil.rmtree, self.root, ignore_errors=True)
|
||||
subprocess.run(
|
||||
["git", "init", "-q", str(root)], check=True, capture_output=True
|
||||
["git", "init", "-q", str(self.root)], check=True, capture_output=True
|
||||
)
|
||||
shutil.copy(REPO_ROOT / ".gitignore", root / ".gitignore")
|
||||
shutil.copy(REPO_ROOT / ".gitignore", self.root / ".gitignore")
|
||||
|
||||
def test_upskill_reports_ignored_at_depth_but_skill_md_stays_tracked(self):
|
||||
# The upskill skill resolves `upskill/` relative to its own directory
|
||||
# (the same observed behavior the **/job_scraper rules exist for), so a
|
||||
# report must be ignored at that depth too. The skill's own SKILL.md
|
||||
# lives in a directory that shares the `upskill` name, so a broad
|
||||
# `**/upskill/*.md` would ignore the template's own skill file - this
|
||||
# pins that it stays tracked.
|
||||
cases = {
|
||||
"upskill/report-2026-08-11.md": True,
|
||||
".claude/skills/upskill/upskill/report-2026-08-11.md": True,
|
||||
@@ -288,7 +292,7 @@ class GitignorePatternBehaviorTests(unittest.TestCase):
|
||||
for path, expect_ignored in cases.items():
|
||||
with self.subTest(path=path):
|
||||
result = subprocess.run(
|
||||
["git", "-C", str(root), "check-ignore", "-q", path],
|
||||
["git", "-C", str(self.root), "check-ignore", "-q", path],
|
||||
capture_output=True,
|
||||
)
|
||||
self.assertEqual(
|
||||
@@ -297,6 +301,38 @@ class GitignorePatternBehaviorTests(unittest.TestCase):
|
||||
f"{path}: expected ignored={expect_ignored}",
|
||||
)
|
||||
|
||||
def test_interview_prep_pack_is_ignored_at_the_path_the_command_writes(self):
|
||||
# Derived, never copied: a hardcoded prep-pack path pins only that
|
||||
# documents/applications/** still matches that shape - which the
|
||||
# presence guard already catches - and stays green if /interview moves
|
||||
# its output, leaving .gitignore's comment stale exactly the way #336
|
||||
# found it. Reading the path back from the command spec is what makes
|
||||
# the move fail here instead.
|
||||
# Two fragments, not one literal: #329 split the path across Step 1
|
||||
# (which derives the archive folder) and Step 3 (which names the file),
|
||||
# so either half can move independently and each must be pinned.
|
||||
folder = "documents/applications/<company>_<role>/"
|
||||
filename = "interview_prep_<stage>.md"
|
||||
spec = (REPO_ROOT / ".claude" / "commands" / "interview.md").read_text(encoding="utf-8")
|
||||
for fragment in (folder, filename):
|
||||
# assertTrue, not assertIn: the haystack is the whole command spec,
|
||||
# and dumping it buries the one sentence explaining the failure.
|
||||
self.assertTrue(
|
||||
fragment in spec,
|
||||
f"/interview no longer writes {fragment}; .gitignore's comment is now stale",
|
||||
)
|
||||
|
||||
path = folder.replace("<company>_<role>", "acme_data_scientist") + filename.replace(
|
||||
"<stage>", "technical"
|
||||
)
|
||||
result = subprocess.run(
|
||||
["git", "-C", str(self.root), "check-ignore", "-v", path],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
self.assertEqual(result.returncode, 0, f"{path}: not ignored by the shipped .gitignore")
|
||||
self.assertIn("documents/applications/**", result.stdout)
|
||||
|
||||
|
||||
class GitignoreNegationTests(GuardRepoFixture):
|
||||
def test_negation_reincluding_personal_data_fails(self):
|
||||
|
||||
@@ -80,6 +80,8 @@ REQUIRED_IGNORE_RULES = [
|
||||
"documents/references/**",
|
||||
"documents/applications/**",
|
||||
"documents/postings/**",
|
||||
# Belt-and-braces, not the primary guard: nothing writes here.
|
||||
# /interview's prep packs land under documents/applications/**, above.
|
||||
"documents/interview/**",
|
||||
"job_search_tracker.csv",
|
||||
"gmail_sync/",
|
||||
|
||||
Reference in New Issue
Block a user