From 30158c0493ae8ae1610769955ad04b189049afe8 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen <50207393+MadsLorentzen@users.noreply.github.com> Date: Sun, 18 Oct 2026 08:16:56 +0200 Subject: [PATCH] chore(guards): ignore node_modules without a trailing slash and fail on any tracked node_modules path (#627) .gitignore's `node_modules/` rule matched directories only, so a symlink named node_modules pointing at a contributor's absolute path was committed (#623), broke `bun install` in CI with a dangling link and checked out as a text file on Windows. The rule loses its slash, and security_guards gains a check that no tracked path has a node_modules component (skipped quietly outside a git checkout). Also qualifies the one bare `seen_jobs.json` mention left in README.md after #617. Co-authored-by: Claude Fable 5.1 --- .gitignore | 4 ++-- CHANGELOG.md | 8 ++++--- README.md | 2 +- tests/test_security_guards.py | 39 +++++++++++++++++++++++++++++++++++ tools/security_guards.py | 32 +++++++++++++++++++++++++++- 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index fb9d9713..b3473abe 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ # Dependencies -node_modules/ +node_modules bun.lock # Python @@ -113,7 +113,7 @@ company_research/*.json # Agent skills: track the source, ignore only deps and logs. # (A blanket `.agents/` ignore silently drops the job-search CLI skills from the repo.) -.agents/**/node_modules/ +.agents/**/node_modules .agents/**/*.log .agents/usage/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f4b1b26..1dfb50b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,12 +68,14 @@ per-file diff commands. ### Fixed -- **`job_key.py --audit` reports a state file that is not an object of entries** (`tools/job_key.py`, +- **`.gitignore` ignores `node_modules` without a trailing slash, and `security_guards.py` fails on any tracked `node_modules` path** (`.gitignore`, `tools/security_guards.py`, `tests/test_security_guards.py`) - the old `node_modules/` rule matched directories only, so a symlink named `node_modules` pointing at a contributor's absolute path was committed in a PR, broke `bun install` in CI with a dangling link and checked out as a text file on Windows. The rule loses its slash, and the guard now lists every tracked path with a `node_modules` component (skipped quietly outside a git checkout). Also qualifies the one bare `seen_jobs.json` mention left in `README.md` after #617. + +- **`job_key.py --audit` reports a state file that is not an object of entries** (#626) (`tools/job_key.py`, `tests/test_job_key.py`) - a top-level JSON array, or an entry that is a string or number, crashed the audit with an `AttributeError` traceback on `.get`, although the function already had a one-line error for an unusable `seen` value. Both now print a one-line message naming the problem and exit 1. -- **`freehire-search` emits the contract's `YYYY-MM-DD` date instead of the API's ISO timestamp** +- **`freehire-search` emits the contract's `YYYY-MM-DD` date instead of the API's ISO timestamp** (#621) (`.agents/skills/freehire-search/cli/src/helpers.ts`, `tests/parsing.test.ts`, `tests/commands.test.ts`, `SKILL.md`) - `toResult` passed `posted_at` through verbatim, so search and detail JSON carried `"date": "2026-07-06T00:00:00Z"` while every other portal CLI @@ -83,7 +85,7 @@ per-file diff commands. its date part and yields `null` for an absent or non-date value; the two tests that pinned the raw timestamp now expect the date and fail on master, plus three cases on the helper. -- **`/rank`, `/notion-sync` and `/upskill` name `job_scraper/seen_jobs.json` by its full path** +- **`/rank`, `/notion-sync` and `/upskill` name `job_scraper/seen_jobs.json` by its full path** (#617) (follow-up to #608, `.claude/commands/rank.md`, `.claude/commands/notion-sync.md`, `.claude/skills/upskill/SKILL.md`, `tests/test_scrape_seen_jobs_path.py`) - #608 qualified the path in `/scrape`, where the file is written; the three readers still said the bare diff --git a/README.md b/README.md index 404649e5..c4199947 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ Postings are treated as untrusted input (the workflow follows no instructions em - **`/gmail-sync`** reads your Gmail (via the Gmail connector) for status signals on your open applications - interview invites, assessment links, offers, rejections - and proposes them as a batch for you to approve before anything is written to the tracker or `outcome.md`, citing the source email on every proposed change. Offers stop short of proposing `hired`/`offer_declined` since that's your call; conflicting or unmatched signals get flagged for a manual `/outcome` pass instead of guessed. - **`/rank`** bridges `/scrape` and `/apply`: it batch-scores all newly scraped postings against the fit framework (parallel agents fetch each posting and score the five evaluation dimensions) and returns a ranked shortlist with honest per-job strengths and gaps. Deal-breakers veto, deadlines get urgency flags, dead postings get marked expired. Pick a number and it hands off to the full `/apply` workflow. - **`/expand`** enriches your profile by scanning public sources you've already linked in it (GitHub repos, portfolio site, Kaggle, Google Scholar) and looking up syllabi for named courses and certifications. Discovered competencies are added to your profile with a source tag. Useful right after `/setup` to surface skills that documents alone don't make explicit. -- **`/upskill`** analyzes the gap between your profile, your tracked job postings, and your ranked-but-untracked postings (`/rank`'s recorded gaps in `seen_jobs.json`) — or a single posting via `/upskill `. Produces a prioritized heatmap of skill gaps and a learning plan with web-searched study resources and time estimates. Useful for career planning between applications. +- **`/upskill`** analyzes the gap between your profile, your tracked job postings, and your ranked-but-untracked postings (`/rank`'s recorded gaps in `job_scraper/seen_jobs.json`) — or a single posting via `/upskill `. Produces a prioritized heatmap of skill gaps and a learning plan with web-searched study resources and time estimates. Useful for career planning between applications. - **`/html-report`** generates a self-contained HTML dashboard from `job_search_tracker.csv` and the application archives — stat cards, status/sector/channel/funnel charts (inline SVG, no external dependencies), and a filterable applications table. Opens directly in a browser, fully offline. Re-run it any time after `/apply` or `/outcome` adds new entries. - **`/add-template`** registers your own CV or cover letter template (LaTeX, Typst, or another toolchain) in place of the stock ones. It captures the template's instructions (source extension, compile command, fonts, style rules, page limit), runs a mandatory test compile, and wires the template into `/apply`. See [Custom templates](#custom-templates) below. - **`/add-portal`** generates a job-portal search skill for a job board in your market. It investigates the portal (search URL pattern, result structure, access rules), scaffolds the CLI skill from the same structure as the shipped ones, and test-runs a live query before registering. See [Job search tools](#job-search-tools) below. diff --git a/tests/test_security_guards.py b/tests/test_security_guards.py index 334450e7..ac561b52 100644 --- a/tests/test_security_guards.py +++ b/tests/test_security_guards.py @@ -416,6 +416,45 @@ class ManifestGuardTests(GuardRepoFixture): self.assertIn("no package.json files found", result.stdout) +class TrackedNodeModulesTests(GuardRepoFixture): + """A committed node_modules entry (symlink, file or directory) fails the guard.""" + + def _git(self, *args): + return subprocess.run( + ["git", *args], cwd=self.root, capture_output=True, text=True, check=False + ) + + def _init_repo(self): + self._git("init", "-q") + self._git("config", "user.email", "t@example.com") + self._git("config", "user.name", "t") + + def test_tracked_node_modules_file_fails(self): + # #623 shape: a file (or symlink, which git stores the same way) named + # node_modules under a CLI directory, slipped past the directory-only rule. + self._init_repo() + cli = self.root / ".agents" / "skills" / "x-search" / "cli" + cli.mkdir(parents=True) + (cli / "node_modules").write_text("/Users/someone/project/node_modules", encoding="utf-8") + self._git("add", "-f", ".agents/skills/x-search/cli/node_modules") + result = run_guards(self.root) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("tracked path inside node_modules", result.stdout) + self.assertIn("x-search/cli/node_modules", result.stdout) + + def test_untracked_node_modules_directory_passes(self): + self._init_repo() + nm = self.root / ".agents" / "skills" / "x-search" / "cli" / "node_modules" / "dep" + nm.mkdir(parents=True) + (nm / "index.js").write_text("", encoding="utf-8") + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_not_a_git_checkout_is_skipped(self): + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + class RealRepoTests(unittest.TestCase): def test_guards_pass_on_this_repo(self): # The live check CI runs: the actual repo tree must satisfy its own guards. diff --git a/tools/security_guards.py b/tools/security_guards.py index c4c7d85c..4190db3b 100644 --- a/tools/security_guards.py +++ b/tools/security_guards.py @@ -28,6 +28,7 @@ Stdlib only. Exit 0 on success, 1 with a failure list otherwise. """ import json +import subprocess import sys from pathlib import Path @@ -279,10 +280,39 @@ def check_package_manifests() -> None: ) +def check_tracked_node_modules() -> None: + """Fail on any tracked path with a ``node_modules`` component. + + ``.gitignore``'s old ``node_modules/`` rule matched directories only, so a + symlink or plain file named ``node_modules`` (a contributor's local symlink + to an absolute path, #623) was committed, broke ``bun install`` in CI with a + dangling link, and checked out as a text file on Windows. The rule now has + no trailing slash; this check catches what the rule cannot, such as a copy + force-added or inherited from before the rule changed. Skipped quietly when + the tree is not a git checkout (the guard also runs from a bare copy). + """ + try: + proc = subprocess.run( + ["git", "ls-files", "-z"], cwd=ROOT, capture_output=True, check=False + ) + except OSError: + return + if proc.returncode != 0: + return + for rel in proc.stdout.decode("utf-8", errors="replace").split("\x00"): + if rel and "node_modules" in rel.split("/"): + errors.append( + f"tracked path inside node_modules: {rel!r}. Installed dependencies " + "(or a symlink standing in for them) must never be committed: run " + "`git rm --cached` on it; .gitignore's `node_modules` rule keeps it out." + ) + + def main() -> int: check_permissions() check_gitignore() check_package_manifests() + check_tracked_node_modules() if errors: print(f"security_guards: {len(errors)} failure(s)") for err in errors: @@ -290,7 +320,7 @@ def main() -> int: return 1 print( "security_guards: OK (permissions allowlist, hooks allowlist, gitignore rules, " - "package manifests)" + "package manifests, no tracked node_modules)" ) return 0