mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-11-02 13:12:50 +00:00
fix(tools): confine verify_pdf --dump-text and drop the bare pdftotext allowance
settings.json pre-approves verify_pdf.py with any arguments, and --dump-text wrote the PDF's text layer to any path it was given, creating missing parent directories and overwriting existing files. Reproduced on master through the real CLI: a one-page PDF's text was written to a new directory outside the repo with no prompt. The CLI now writes only a .txt under the repo's cv/ or cover_letters/ (where apply.md Step 5d puts it) and refuses a symlinked target. Bash(pdftotext:*) is dropped for the same reason: Poppler writes to any output path, and `pdftotext <file> -` prints any PDF on disk into the session. verify_pdf.py already wraps it; the documented manual fallback still works and now asks first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9ca456318c
commit
bc2c583c44
7 files changed
+91
-5
No files matched your search
@@ -20,7 +20,6 @@
|
|||||||
"Bash(python3 tools/verify_pdf.py:*)",
|
"Bash(python3 tools/verify_pdf.py:*)",
|
||||||
"Bash(python tools/verify_layout.py:*)",
|
"Bash(python tools/verify_layout.py:*)",
|
||||||
"Bash(python3 tools/verify_layout.py:*)",
|
"Bash(python3 tools/verify_layout.py:*)",
|
||||||
"Bash(pdftotext:*)",
|
|
||||||
"Bash(python tools/robots_check.py:*)",
|
"Bash(python tools/robots_check.py:*)",
|
||||||
"Bash(python3 tools/robots_check.py:*)",
|
"Bash(python3 tools/robots_check.py:*)",
|
||||||
"Bash(python tools/fetch_page.py:*)",
|
"Bash(python tools/fetch_page.py:*)",
|
||||||
|
|||||||
@@ -493,6 +493,18 @@ per-file diff commands.
|
|||||||
template ships no hooks; only settings.json hooks were checked). Check 5 refuses an agent
|
template ships no hooks; only settings.json hooks were checked). Check 5 refuses an agent
|
||||||
`permissionMode` that removes prompts and agent `hooks` or `mcpServers`.
|
`permissionMode` that removes prompts and agent `hooks` or `mcpServers`.
|
||||||
|
|
||||||
|
- **Pre-approved PDF tooling can no longer write to arbitrary paths** (`tools/verify_pdf.py`,
|
||||||
|
`.claude/settings.json`, `tools/security_guards.py`, `tests/test_verify_pdf.py`) -
|
||||||
|
`settings.json` pre-approves `verify_pdf.py` with any arguments, and `--dump-text` wrote the
|
||||||
|
extracted text layer to whatever path it was given, creating missing parent directories and
|
||||||
|
overwriting an existing file: a prompt-free write anywhere on disk, with content taken from a
|
||||||
|
PDF that can carry posting-derived text. The CLI now writes only a `.txt` file under the repo's
|
||||||
|
`cv/` or `cover_letters/` (the place `apply.md` Step 5d documents) and refuses a symlinked
|
||||||
|
target. `Bash(pdftotext:*)` is dropped from the allowlist for the same reason - Poppler writes
|
||||||
|
to any output path and `pdftotext <file> -` prints any PDF on disk into the session - and
|
||||||
|
`verify_pdf.py` already wraps it. The documented manual `pdftotext` fallback in Step 5d still
|
||||||
|
works; it now asks first.
|
||||||
|
|
||||||
## [1.7.3] - 2026-10-19
|
## [1.7.3] - 2026-10-19
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
+1
-1
@@ -20,7 +20,7 @@ Every grant the template ships, where it lives, how long it lasts, and what chec
|
|||||||
|
|
||||||
| Surface | What is granted | Lifetime | Guard |
|
| Surface | What is granted | Lifetime | Guard |
|
||||||
|---|---|---|---|
|
|---|---|---|---|
|
||||||
| `.claude/settings.json` `permissions.allow` | Each shipped portal CLI by exact path, `salary_lookup.py`, the `tools/` scripts (`rank_state`, `job_key`, `verify_pdf`, `verify_layout`, `robots_check`, `fetch_page`), `pdftotext`, and `Skill(job-application-assistant)` | Whole session | Exact allowlist; any addition fails CI unless added to `ALLOWED_PERMISSIONS` in the same PR |
|
| `.claude/settings.json` `permissions.allow` | Each shipped portal CLI by exact path, `salary_lookup.py`, the `tools/` scripts (`rank_state`, `job_key`, `verify_pdf`, `verify_layout`, `robots_check`, `fetch_page`), and `Skill(job-application-assistant)` | Whole session | Exact allowlist; any addition fails CI unless added to `ALLOWED_PERMISSIONS` in the same PR |
|
||||||
| `.claude/settings.json` `permissions.ask` | Always prompt, even over an allow rule or a skill grant: edits to `settings.json`, `settings.local.json`, any `.claude/skills/*/SKILL.md`, any `.claude/commands/*.md`, `.claude/agents/**`, `.github/**`, `tools/security_guards.py`; edits to the files the allow list runs by path (`tools/**`, `.agents/**`, `salary_lookup.py`); and `git push` | Whole session | Each rule is required; removal fails CI |
|
| `.claude/settings.json` `permissions.ask` | Always prompt, even over an allow rule or a skill grant: edits to `settings.json`, `settings.local.json`, any `.claude/skills/*/SKILL.md`, any `.claude/commands/*.md`, `.claude/agents/**`, `.github/**`, `tools/security_guards.py`; edits to the files the allow list runs by path (`tools/**`, `.agents/**`, `salary_lookup.py`); and `git push` | Whole session | Each rule is required; removal fails CI |
|
||||||
| `.claude/settings.json` `permissions.deny` | Blocked in every mode, `bypassPermissions` included: `curl`/`wget` with data, form or upload flags, `scp`, `sftp`, `ssh`, `nc`; reads of `.env`, `.env.*`, `~/.ssh/**`, `~/.aws/**`, `~/.gnupg/**` (Read denies also cover Bash `cat`/`head`/`tail`/`sed`) | Whole session | Each rule is required; removal fails CI |
|
| `.claude/settings.json` `permissions.deny` | Blocked in every mode, `bypassPermissions` included: `curl`/`wget` with data, form or upload flags, `scp`, `sftp`, `ssh`, `nc`; reads of `.env`, `.env.*`, `~/.ssh/**`, `~/.aws/**`, `~/.gnupg/**` (Read denies also cover Bash `cat`/`head`/`tail`/`sed`) | Whole session | Each rule is required; removal fails CI |
|
||||||
| Skill `allowed-tools` (`.claude/skills/*/SKILL.md`) | `job-application-assistant`: Read, Glob, Grep, WebFetch, WebSearch, Edit, Write, AskUserQuestion. `scrape`: the same plus Agent, `bun --version`, `tools/job_key.py`. `upskill`: Read, Write, Glob, Grep, WebFetch, WebSearch. No bare `Bash` anywhere. | The turn that invokes the skill; clears on your next message. Not gated by workspace trust. | Exact allowlist (`ALLOWED_SKILL_TOOLS`); bare tools and wildcards fail CI |
|
| Skill `allowed-tools` (`.claude/skills/*/SKILL.md`) | `job-application-assistant`: Read, Glob, Grep, WebFetch, WebSearch, Edit, Write, AskUserQuestion. `scrape`: the same plus Agent, `bun --version`, `tools/job_key.py`. `upskill`: Read, Write, Glob, Grep, WebFetch, WebSearch. No bare `Bash` anywhere. | The turn that invokes the skill; clears on your next message. Not gated by workspace trust. | Exact allowlist (`ALLOWED_SKILL_TOOLS`); bare tools and wildcards fail CI |
|
||||||
|
|||||||
@@ -344,7 +344,7 @@ Make sure Bun is installed and you ran `bun install` in each CLI directory. The
|
|||||||
The cover letter template expects fonts in `cover_letters/OpenFonts/fonts/`. Make sure this directory exists and contains the Lato and Raleway font files.
|
The cover letter template expects fonts in `cover_letters/OpenFonts/fonts/`. Make sure this directory exists and contains the Lato and Raleway font files.
|
||||||
|
|
||||||
### Stale `.claude/settings.local.json` from an older clone
|
### Stale `.claude/settings.local.json` from an older clone
|
||||||
Shared Claude Code permissions now live in `.claude/settings.json`: an `allow` list scoped to the exact commands the workflow runs (each portal CLI by path, the `tools/` scripts, `salary_lookup.py`, `pdftotext`), an `ask` list that always prompts before the permission-defining files are edited or `git push` runs, and a `deny` list that blocks upload-shaped shell and reads of credential files in every mode. `SECURITY.md` has the full table. Earlier versions of this repo committed a broader `.claude/settings.local.json` that pre-approved `Bash(curl:*)`, `Bash(python:*)` and `Bash(bun:*)`. If you cloned before that change, git leaves the old file behind in your working copy, and its permissions still apply on top of `settings.json`. Delete it (or trim it to your own personal overrides):
|
Shared Claude Code permissions now live in `.claude/settings.json`: an `allow` list scoped to the exact commands the workflow runs (each portal CLI by path, the `tools/` scripts and `salary_lookup.py`), an `ask` list that always prompts before the permission-defining files are edited or `git push` runs, and a `deny` list that blocks upload-shaped shell and reads of credential files in every mode. `SECURITY.md` has the full table. Earlier versions of this repo committed a broader `.claude/settings.local.json` that pre-approved `Bash(curl:*)`, `Bash(python:*)` and `Bash(bun:*)`. If you cloned before that change, git leaves the old file behind in your working copy, and its permissions still apply on top of `settings.json`. Delete it (or trim it to your own personal overrides):
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
rm .claude/settings.local.json
|
rm .claude/settings.local.json
|
||||||
|
|||||||
@@ -5,12 +5,15 @@ from pathlib import Path
|
|||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
from tools.verify_pdf import (
|
from tools.verify_pdf import (
|
||||||
|
ROOT,
|
||||||
VerificationError,
|
VerificationError,
|
||||||
build_parser,
|
build_parser,
|
||||||
|
check_dump_target,
|
||||||
extract_text_layer,
|
extract_text_layer,
|
||||||
find_non_ascii_date_ranges,
|
find_non_ascii_date_ranges,
|
||||||
normalize_text,
|
normalize_text,
|
||||||
parse_page_count,
|
parse_page_count,
|
||||||
|
main,
|
||||||
run_tool,
|
run_tool,
|
||||||
verify_pdf,
|
verify_pdf,
|
||||||
)
|
)
|
||||||
@@ -331,5 +334,48 @@ class RunToolTests(unittest.TestCase):
|
|||||||
run_tool(["pdfinfo", "example.pdf"])
|
run_tool(["pdfinfo", "example.pdf"])
|
||||||
|
|
||||||
|
|
||||||
|
class DumpTargetTests(unittest.TestCase):
|
||||||
|
"""`.claude/settings.json` pre-approves verify_pdf.py with any arguments, so
|
||||||
|
`--dump-text` was a prompt-free write to any path on disk (it also created
|
||||||
|
missing parent directories). The CLI now writes only a .txt under the
|
||||||
|
repo's cv/ or cover_letters/, the place apply.md Step 5d documents."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.temp_dir = tempfile.TemporaryDirectory()
|
||||||
|
self.addCleanup(self.temp_dir.cleanup)
|
||||||
|
self.root = Path(self.temp_dir.name)
|
||||||
|
(self.root / "cv").mkdir()
|
||||||
|
|
||||||
|
def test_documented_target_is_allowed(self):
|
||||||
|
self.assertIsNone(check_dump_target(self.root / "cv" / "main_acme_analyst.txt", self.root))
|
||||||
|
self.assertIsNone(check_dump_target(ROOT / "cv" / "main_acme_analyst.txt"))
|
||||||
|
|
||||||
|
def test_targets_outside_the_output_dirs_are_refused(self):
|
||||||
|
outside = Path(self.temp_dir.name).parent / "elsewhere.txt"
|
||||||
|
for path, reason in [
|
||||||
|
(outside, "inside this repo's cv/"),
|
||||||
|
(self.root / "tools" / "notes.txt", "inside this repo's cv/"),
|
||||||
|
(self.root / "cv" / ".." / "tools" / "x.txt", "inside this repo's cv/"),
|
||||||
|
(self.root / "cv" / "main_acme.py", "must name a .txt file"),
|
||||||
|
(Path.home() / ".zshrc", "must name a .txt file"),
|
||||||
|
]:
|
||||||
|
with self.subTest(path=path):
|
||||||
|
self.assertIn(reason, check_dump_target(path, self.root))
|
||||||
|
|
||||||
|
def test_symlink_out_of_cv_is_refused(self):
|
||||||
|
link = self.root / "cv" / "main_acme.txt"
|
||||||
|
link.symlink_to(self.root / "outside.txt")
|
||||||
|
self.assertEqual(check_dump_target(link, self.root), "must not be a symlink")
|
||||||
|
|
||||||
|
@patch("tools.verify_pdf.verify_pdf")
|
||||||
|
def test_cli_refuses_before_extracting_or_writing(self, mock_verify):
|
||||||
|
target = Path(self.temp_dir.name) / "victim" / "rc.txt"
|
||||||
|
with patch("sys.stderr"):
|
||||||
|
code = main(["cv/main_example.pdf", "--dump-text", str(target)])
|
||||||
|
self.assertEqual(code, 1)
|
||||||
|
mock_verify.assert_not_called()
|
||||||
|
self.assertFalse(target.parent.exists(), "the CLI must not create the parent directory")
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
@@ -82,7 +82,8 @@ ALLOWED_PERMISSIONS = {
|
|||||||
"Bash(python3 tools/verify_pdf.py:*)",
|
"Bash(python3 tools/verify_pdf.py:*)",
|
||||||
"Bash(python tools/verify_layout.py:*)",
|
"Bash(python tools/verify_layout.py:*)",
|
||||||
"Bash(python3 tools/verify_layout.py:*)",
|
"Bash(python3 tools/verify_layout.py:*)",
|
||||||
"Bash(pdftotext:*)",
|
# No bare Bash(pdftotext:*): it writes its output to any path it is given,
|
||||||
|
# and tools/verify_pdf.py already wraps it with that path confined.
|
||||||
# The browser-header retry in 09-web-research.md. fetch_page.py is GET-only
|
# The browser-header retry in 09-web-research.md. fetch_page.py is GET-only
|
||||||
# with a fixed argv and refuses to write inside the repo, which is why the
|
# with a fixed argv and refuses to write inside the repo, which is why the
|
||||||
# grant names the script and not `curl`.
|
# grant names the script and not `curl`.
|
||||||
|
|||||||
+29
-1
@@ -24,10 +24,33 @@ import unicodedata
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
|
ROOT = Path(__file__).resolve().parent.parent
|
||||||
|
|
||||||
|
# `.claude/settings.json` pre-approves this tool with any arguments, so where
|
||||||
|
# `--dump-text` may write is part of that permission. The workflow only dumps a
|
||||||
|
# CV's text layer beside it (`apply.md` Step 5d writes `cv/main_<company>_<role>.txt`);
|
||||||
|
# a path anywhere else - a shell rc file, a script the allowlist runs - would be
|
||||||
|
# an overwrite nobody was asked about.
|
||||||
|
DUMP_TEXT_DIRS = ("cv", "cover_letters")
|
||||||
|
|
||||||
|
|
||||||
class VerificationError(Exception):
|
class VerificationError(Exception):
|
||||||
"""Raised when a generated PDF does not satisfy its checks."""
|
"""Raised when a generated PDF does not satisfy its checks."""
|
||||||
|
|
||||||
|
|
||||||
|
def check_dump_target(path, root=ROOT):
|
||||||
|
"""Return why `--dump-text` may not write to `path`, or None if it may."""
|
||||||
|
path = Path(path)
|
||||||
|
if path.suffix != ".txt":
|
||||||
|
return "must name a .txt file"
|
||||||
|
if path.is_symlink():
|
||||||
|
return "must not be a symlink"
|
||||||
|
parent = path.resolve().parent
|
||||||
|
if not any(parent.is_relative_to((root / d).resolve()) for d in DUMP_TEXT_DIRS):
|
||||||
|
return "must be inside this repo's " + " or ".join(f"{d}/" for d in DUMP_TEXT_DIRS)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def run_tool(command):
|
def run_tool(command):
|
||||||
try:
|
try:
|
||||||
return subprocess.run(
|
return subprocess.run(
|
||||||
@@ -302,7 +325,7 @@ def build_parser():
|
|||||||
parser.add_argument(
|
parser.add_argument(
|
||||||
"--dump-text",
|
"--dump-text",
|
||||||
type=Path,
|
type=Path,
|
||||||
help="write the extracted text layer to this path (UTF-8)",
|
help="write the extracted text layer to this .txt path under cv/ or cover_letters/ (UTF-8)",
|
||||||
)
|
)
|
||||||
parser.add_argument(
|
parser.add_argument(
|
||||||
"--ascii-dates",
|
"--ascii-dates",
|
||||||
@@ -332,6 +355,11 @@ def _force_utf8_output() -> None:
|
|||||||
def main(argv=None):
|
def main(argv=None):
|
||||||
_force_utf8_output()
|
_force_utf8_output()
|
||||||
args = build_parser().parse_args(argv)
|
args = build_parser().parse_args(argv)
|
||||||
|
if args.dump_text is not None:
|
||||||
|
reason = check_dump_target(args.dump_text)
|
||||||
|
if reason:
|
||||||
|
print(f"Error: --dump-text {args.dump_text}: {reason}", file=sys.stderr)
|
||||||
|
return 1
|
||||||
try:
|
try:
|
||||||
extractor, text, pages = verify_pdf(
|
extractor, text, pages = verify_pdf(
|
||||||
args.pdf,
|
args.pdf,
|
||||||
|
|||||||
Reference in new issue
Block a user