Skip to content

fix: security & reliability hardening from the 2026-07-10 review (P2-1, P0-4, P0-3, P1-7)#101

Merged
shoom1 merged 22 commits into
developfrom
fix/review-hardening-2026-07
Jul 11, 2026
Merged

fix: security & reliability hardening from the 2026-07-10 review (P2-1, P0-4, P0-3, P1-7)#101
shoom1 merged 22 commits into
developfrom
fix/review-hardening-2026-07

Conversation

@shoom1

@shoom1 shoom1 commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

Security & reliability hardening for the P2-1, P0-4, P0-3, and P1-7 findings from the 2026-07-10 comprehensive review — dependency declaration, glob/grep containment, host LaTeX compilation, and permission fail-closed behavior. 22 commits; offline suite 1908 passed.

What's fixed

  • P2-1 — packaging. Declare html2text (eagerly imported by the webfetch tools) in pyproject.toml so a clean pip install doesn't break on first import; add a packaging regression test.
  • P0-4 — glob/grep containment. The permission engine authorizes only path, but ../* / absolute patterns escaped it. Reject ../absolute patterns and containment-filter results in glob and both grep backends (Python + ripgrep, with a RIPGREP_CONFIG_PATH scrub); filter hidden path components; add pre-limit traversal ceilings (_MAX_SCAN; grep _MAX_FILES/_MAX_FILE_BYTES, directory-safe and truncation-reporting); bound ripgrep output.
  • P0-3 — host TeX execution (kept host-side, decoupled from the container sandbox by design):
    • anchor option-like source_path basenames with ./ (a -pdflatex=CMD.tex name was parsed as an engine option → arbitrary host exec);
    • latexmk -norc + pdflatex -no-shell-escape (no .latexmkrc Perl, no \write18);
    • exact TeX environment allowlist (no host secrets leak to the subprocess); reject assets_dir path-list injection;
    • compile in a private temp build dir (intermediates never touch the source dir); no-follow atomic PDF delivery;
    • bounded .log tail-read + bounded subprocess capture; RLIMIT_FSIZE/RLIMIT_CPU on the child;
    • scoped filesystem.read/write capabilities via a new Capability.optional flag.
  • P1-7 — permission fail-closed. The ADK, LangGraph, and MCP adapters denied-on-missing-engine only when permissions are disabled; now they deny when permissions are enabled but the engine is absent (a misconfiguration), and allow only when permissions are off.

Verification

  • Offline suite: 1908 passed, 1 skipped (gated LaTeX), 26 xfailed.
  • Built via subagent-driven development (per-task TDD + spec/quality review), an opus whole-branch review (all findings applied), and a 17/17 independent reproduction harness that re-runs each original finding's exploit/scenario against the final code and confirms it is blocked.

Not in scope (deferred, with rationale)

  • RLIMIT_AS/RLIMIT_NPROC for the TeX child (address-space caps risk legit large docs; NPROC risks latexmk→pdflatex/biber; the timeout + process-group SIGKILL + RLIMIT_FSIZE/CPU already bound runaways).
  • Multi-file / BibTeX / \include{subdir/…} under the private build dir — a feature-scope limitation; the supported shape is a single main .tex + resources via TEXINPUTS.
  • Tier-3 (P0-1 config trust model, P0-2 sandbox symlink-safe transfer, P0-5 SSRF transport) and the LangGraph P1s (retirement pending) — separate release cycles.

https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv

shoom1 added 22 commits July 10, 2026 17:07
@shoom1
fix(deps): declare html2text in pyproject (P2-1)
767c89f 
converter.py imports html2text at module load, but it was only in
environment.yml — a clean pip install broke on first webfetch import.
Add it to core dependencies + a packaging test asserting it's declared.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(tools): contain glob/grep patterns to the authorized root (P0-4)
8be9f70 
The permission engine authorizes only the `path` argument, but glob ran
`search_path.glob(pattern)` and grep's Python fallback ran
`path.glob(file_pattern)` — a pattern of `../*` or an absolute path
escaped the granted directory (reproduced against a sibling dir).

Reject absolute/`..` patterns at the source in both tools, and
containment-check each resolved result (drops symlinks under the root
that point outside). Shared helpers glob_pattern_escapes_root /
path_is_within added to file_utils.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): harden compile_document host execution (P0-3)
a1a45bb 
Three host-side gaps closed, keeping compilation decoupled from the
container sandbox:

- output_pdf delivery used shutil.copy2, which follows a destination
  symlink -> write-through-symlink primitive. Now a no-follow atomic
  write (temp file in dest dir + os.replace); a symlink at the output
  path is replaced, never written through.
- the TeX subprocess inherited the full host environment (secrets, API
  keys) -> now an allowlisted env (PATH/HOME/TEXINPUTS/locale only).
- the capability covered only source_path -> added optional
  filesystem.read (assets_dir) and filesystem.write (output_pdf).

Adds a Capability.optional flag: an optional target arg that's absent/
empty is skipped in the engine's _resolve, so scoping the extra caps
doesn't spuriously prompt when those args aren't supplied. latexmk
stays the default engine (.latexmkrc risk remains a documented
deferral).

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(permissions): fail closed when the engine is missing (P1-7)
3a4434a 
Both adapters (ADK PermissionPlugin, LangGraph wrap_tool_for_permission)
treated an absent PermissionEngine as allow. base_manager always
constructs the engine, so a missing engine with permissions enabled is a
misconfiguration — allowing silently bypasses all gating.

Now: engine absent + permissions_enabled -> deny; engine absent +
permissions disabled -> allow (matches the master switch). Also covers
the MCP synthetic-capability path. The three test_engine_absent_allows
tests are rewritten to pin both halves of the new semantic.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
refine(document): keep TeX config vars in the scrubbed env (P0-3)
3cfdc7e 
The env allowlist was too aggressive — dropping TEXMFHOME/TEXMFVAR/etc.
would break users with a custom TeX tree. Pass through TeX's own TEX*
config vars (still scrubbing host secrets, still never inheriting the
caller's TEXINPUTS).

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(tools): apply grep containment on the ripgrep path too (P0-4 review)
6e33620 
Review found the symlink-containment filter was only added to grep's
Python fallback; the ripgrep path (the common one when rg is installed)
returned rg's file paths verbatim. A followed symlink (e.g. via a host
RIPGREP_CONFIG_PATH injecting --follow) could surface outside-root file
content under an inside-looking path.

Filter rg's reported files through path_is_within (resolving rg's
relative paths against the search root), and drop RIPGREP_CONFIG_PATH
from the rg subprocess env so the --follow vector is removed at source.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(review): address minor findings from the hardening review
75ac08f 
- engine.check: guard empty outcomes (all-optional caps absent) — was a
  latent IndexError on outcomes[0]; now allows (nothing to gate).
- compile_document: pass PERL5LIB/PERLLIB (latexmk is Perl) so the env
  scrub can't break module loading; preserve the delivered PDF's mode
  via copystat (mkstemp is 0600) so reports stay readable; document that
  no-follow delivery guards only the final path component (parent-dir
  symlink residual, deferred to spec §9).
- test_packaging: scope the docstring to a regression pin, not an
  exhaustive import audit.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(review2): close residuals from the branch re-review (P0-3, P0-4)
d15b9db 
Re-review of the hardening branch reproduced defects in the new code:

P0-3 compile_document:
- _build_env used k.startswith('TEX'), leaking non-TeX vars like
  TEXT_API_TOKEN to the subprocess — defeating the very allowlist. Scope
  to TEXMF* + a fixed set of TeX vars.
- assets_dir containing os.pathsep (e.g. 'assets:/etc') was authorized as
  one filesystem.read target but became multiple TEXINPUTS roots. Reject it.
- Run latexmk with -norc so a build-dir/home .latexmkrc (arbitrary Perl)
  is never executed — the two arbitrary-code vectors (rc + shell-escape)
  are now both off by default. Docstrings/description updated to match.

P0-4 glob:
- include_hidden=False only checked the basename, so '**/*' still returned
  files under a dot-directory (.hidden/secret.txt). Reject any result with
  a hidden component in its path relative to the root.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): anchor option-like source filename (P0-3 critical)
c33ff74 
A source_path whose basename begins with '-' was appended to the engine
argv verbatim, so latexmk/pdflatex parsed it as an OPTION — e.g.
'-pdflatex=CMD.tex', '-r', '-use-make' execute arbitrary commands/Perl.
-norc does not close this: if the option consumes the sole source arg,
latexmk defaults to compiling every *.tex in the dir. source_path is
model-controlled, so this is an arbitrary-host-exec vector.

Anchor the source with './' (_safe_source_arg) for both engines so a
leading '-' is always parsed as a path, never an option.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): build in a private temp dir, isolate intermediates (P0-3)
bdcb544 
Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): tail-read the compiler .log instead of whole file (P0-3)
4e7d1f3 
Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
test(document_compile): add OSError coverage and tighten log-tail bound
62c6c9d 
- Fix 1: Add test_read_log_tail_oserror_returns_fallback to verify _read_log_tail
  returns fallback on any OSError (PermissionError, etc.) without propagating
- Fix 2: Change test_read_log_tail_bounds_large_log bound from +16 to +3
  (decode can only shorten, never lengthen; +3 covers one UTF-8 replacement char)

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): cap captured subprocess output in _run (P0-3)
9831277 
Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document.compile): assert SIGKILL on timeout, rename capture-limi…
72e35ed 
…t to bytes, tighten test slack

Fix 1 (Important): Assert SIGKILL signal in test_run_group_kill_on_timeout.
Fix 2 (Minor): Rename _MAX_CAPTURE_CHARS→_MAX_CAPTURE_BYTES (it's a byte limit).
Fix 3 (Minor): Tighten test_run_caps_captured_output slack from +8 to +1 byte.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): exact TEXMF env allowlist instead of prefix (P0-3)
e4ad61a 
Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(tools): pre-limit scan/size ceilings for glob and grep (P0-4)
a5d6fd3 
Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
chore: stop tracking AGENTS.md
59a45b8 
Swept into 72e35ed by a 'git add -A'; it was meant to stay an untracked
local file. Remove from the tree (kept on disk, untracked).

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document.compile): absolute TEXINPUTS roots, ignore_cleanup_error…
51da5e5 
…s, delivery pdf_path=None

I-1: resolve assets_dir/source_dir to absolute before setting TEXINPUTS so
relative paths don't mis-resolve when the TeX engine runs in a private temp
build dir.
M-1: TemporaryDirectory(ignore_cleanup_errors=True) to guard rare OSError on
cleanup.
M-2: delivery-failure return now sets pdf_path=None (the temp dir is gone on
return); update matching test assertion.
T1: fix output_pdf docstring clause ("isolated in a private temp dir").

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
test(document): fix gated LaTeX test for the temp-build isolation con…
92ade46 
…tract

test_document_compile_latex.py asserted the .log stays in the source dir;
after the private-temp-build change intermediates are isolated (temp dir,
cleaned up), so on a real engine that assertion would fail. The test is
@pytest.mark.latex (skipped here), so it slipped past the offline suite.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(grep): don't let directories consume the file budget; report scan…
2dd8b15 
… truncation (P0-4)

_grep_python counted directories against _MAX_FILES and never set
truncated when the cap was hit, so N directories before a matching file
returned no matches with truncated=False — a silently-wrong result.
Count only files against the budget and set truncated when the file cap
is reached. Also tighten the glob scan-cap test to an exact count.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(document): cap TeX child file size and CPU (RLIMIT_FSIZE/CPU) (P0-3)
2ea7573 
_run bounded retained memory but the child could still fill host disk or
burn CPU within the wall-clock timeout. Add a preexec_fn that sets
RLIMIT_FSIZE (caps any file the child writes, incl. the redirected
stdout/stderr temp files) and RLIMIT_CPU (backstop to the timeout).
Best-effort; POSIX.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1
fix(grep): bound ripgrep output to a temp file + capped read (P0-4)
22aa503 
_grep_with_ripgrep used subprocess.run(capture_output=True) — a large
tree could allocate unbounded JSON in host memory (rg's --max-count is
per-file). Capture rg stdout to a temp file, read back at most
_MAX_RG_OUTPUT_BYTES, and flag truncation; preserve the timeout +
process-group kill and the RIPGREP_CONFIG_PATH scrub.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1 shoom1 merged commit 4c0d481 into develop Jul 11, 2026
2 checks passed
@shoom1 shoom1 deleted the fix/review-hardening-2026-07 branch July 11, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@shoom1