From cfd9a9fba11848afac7ffb0f67944e594871892a Mon Sep 17 00:00:00 2001 From: Ayobami Adegoke Date: Tue, 11 Aug 2026 18:07:21 +0100 Subject: [PATCH] fix(security): gitignore /upskill reports at the path the skill writes them (#317) The upskill/*.md rule is rooted, but /upskill is a skill and skills resolve bare relative paths against their own directory - the same observed behavior the **/job_scraper rules exist for. A report at .claude/skills/upskill/upskill/report-*.md was not ignored, and an upskill report records the candidate's skill gaps against named employers. **/upskill/*.md would also ignore the skill's own SKILL.md (the directory shares the name), so the new rule pins the report-file prefix: **/upskill/report-*.md. Added to .gitignore and REQUIRED_IGNORE_RULES, with a check-ignore-based test pinning both properties. --- .gitignore | 8 ++++++- CHANGELOG.md | 14 ++++++++++++ tests/test_security_guards.py | 41 ++++++++++++++++++++++++++++++++++- tools/security_guards.py | 7 ++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 95d6792..6fa805d 100644 --- a/.gitignore +++ b/.gitignore @@ -89,8 +89,14 @@ gmail_sync/ # Generated reports (personal output from /html-report) reports/ -# Upskill reports (personal output) +# Upskill reports (personal output). Depth-independent like the job_scraper +# rules above: the upskill skill resolves `upskill/` relative to its own +# directory, so a report can land at .claude/skills/upskill/upskill/*.md +# where the rooted rule cannot see it. `**/upskill/*.md` is not usable here - +# the skill directory shares the `upskill` name, so it would also ignore the +# skill's own SKILL.md - hence the report-file prefix is pinned instead. upskill/*.md +**/upskill/report-*.md # Agent skills: track the source, ignore only deps and logs. # (A blanket `.agents/` ignore silently drops the job-search CLI skills from the repo.) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4868d03..b8f6fe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,20 @@ per-file diff commands. ### Fixed +- **`/upskill` reports are now gitignored at the path the skill actually writes them to.** + The ignore rule `upskill/*.md` is rooted (a middle slash anchors a gitignore pattern to the + repo root), but `/upskill` is a *skill*, and skills resolve bare relative paths against + their own directory - the same observed behavior the `**/job_scraper/*` rules exist for. + A report written to `.claude/skills/upskill/upskill/report-*.md` was therefore not ignored + (`git check-ignore` confirms it on the unpatched tree), and an upskill report is the + candidate's skill gaps and weaknesses measured against named employers - among the most + sensitive files the workflow generates. The obvious widening, `**/upskill/*.md`, would have + ignored the template's own `.claude/skills/upskill/SKILL.md` (the skill directory shares + the name), so the new rule pins the report-file prefix instead: `**/upskill/report-*.md`. + Added to `.gitignore` and `security_guards.py`'s `REQUIRED_IGNORE_RULES`, with a + `check-ignore`-based test pinning both properties - reports ignored at both depths, + `SKILL.md` still tracked - which presence checks alone cannot see. + - **Dropped the phantom `evaluated` value from `seen_jobs.json`'s status vocabulary** (#315). The schema block in the job-scraper skill documented `new/skipped/evaluated/ranked/expired`, but `evaluated` has had no writer and no reader since the initial release - `new`/`skipped` diff --git a/tests/test_security_guards.py b/tests/test_security_guards.py index d58519e..dd10061 100644 --- a/tests/test_security_guards.py +++ b/tests/test_security_guards.py @@ -244,7 +244,7 @@ class GitignoreGuardTests(GuardRepoFixture): def test_generated_report_rules_are_required(self): # Reports are generated from the user's tracker and application archive, # so losing these ignore rules can expose personal job-search history. - sensitive_outputs = ["reports/", "upskill/*.md"] + sensitive_outputs = ["reports/", "upskill/*.md", "**/upskill/report-*.md"] remaining = [ rule for rule in security_guards.REQUIRED_IGNORE_RULES @@ -257,6 +257,45 @@ class GitignoreGuardTests(GuardRepoFixture): self.assertEqual(result.returncode, 1) self.assertIn("reports/", result.stdout) self.assertIn("upskill/*.md", result.stdout) + self.assertIn("**/upskill/report-*.md", result.stdout) + + +class GitignorePatternBehaviorTests(unittest.TestCase): + """Pin the match semantics of the shipped .gitignore for upskill reports. + + 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. + """ + + 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) + subprocess.run( + ["git", "init", "-q", str(root)], check=True, capture_output=True + ) + shutil.copy(REPO_ROOT / ".gitignore", root / ".gitignore") + cases = { + "upskill/report-2026-08-11.md": True, + ".claude/skills/upskill/upskill/report-2026-08-11.md": True, + ".claude/skills/upskill/upskill/report-2026-08-11-acme-engineer.md": True, + ".claude/skills/upskill/SKILL.md": False, + } + for path, expect_ignored in cases.items(): + with self.subTest(path=path): + result = subprocess.run( + ["git", "-C", str(root), "check-ignore", "-q", path], + capture_output=True, + ) + self.assertEqual( + result.returncode == 0, + expect_ignored, + f"{path}: expected ignored={expect_ignored}", + ) class GitignoreNegationTests(GuardRepoFixture): diff --git a/tools/security_guards.py b/tools/security_guards.py index b307af7..2111781 100644 --- a/tools/security_guards.py +++ b/tools/security_guards.py @@ -73,6 +73,13 @@ REQUIRED_IGNORE_RULES = [ "gmail_sync/", "reports/", "upskill/*.md", + # Depth-independent twin of the rule above. The upskill *skill* resolves + # `upskill/` relative to its own directory - the same observed behavior + # the **/job_scraper rules exist for - so reports can land at + # .claude/skills/upskill/upskill/*.md where the rooted rule cannot see + # them. `**/upskill/*.md` would also ignore the skill's own SKILL.md + # (the directory shares the name), so the report-file prefix is pinned. + "**/upskill/report-*.md", # Not personal data but the same failure mode: /add-portal can generate a # skill for a portal that only returns usable content through a paid # fetching service, and that skill reads an API token from the environment.