Repository navigation
fix(security): fail closed in deps, hook and git; bound the dashboard, HTTP and regex paths - #70
Merged
Merged
Conversation
findSecretsInText now walks every match of each rule and skips placeholder values one match at a time, so a placeholder no longer hides a real secret of the same rule later in the text. Hits carry their index; at most 50 hits per rule are reported. The repo walker only collects regular files and reads them with a non-blocking open and an fstat check, so a FIFO or device in the tree is never read. Files over 16 MB are reported as an info finding instead of being read. The repo scan still emits one finding per rule and file, so finding ids are unchanged. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the per-match scan, regular-file walker and tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
…files - Staged entries come from `git diff --cached --raw -z` with --diff-filter=ACMRT, and blobs are read by object id through `git cat-file --batch`, so any file name (including newlines or revision-like syntax) and symlink-to-file typechanges are checked. Staged symlinks and submodules are still not followed. - Text containing NUL bytes is scanned. - A staged file over the 16 MB read limit blocks the commit with a critical finding and a warning naming the file; the new policy option hook.largeFiles: "warn" only warns. - hook install lstats .git/hooks/pre-commit: a symlink is never treated as ours, it is moved aside as the chained backup, and the hook is created with flag "wx". Uninstall restores a symlinked backup as it was. - fixHint extracts the first sentence without backtracking regexes. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the hook plumbing, install and fixHint fixes and regression tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
scanPackage now fails closed when a source lists fewer files than the package has, or a code file is over the scan size cap: the package is reported "could not be scanned" (high) and penthera install holds it for approval. Both caps now match the tarball reader's own limits (50,000 entries, 64 MB per file), so real packages such as typescript (a 9 MB typescript.js) and icon or locale packages with many thousands of files are scanned whole rather than held. The tar reader also refuses a pax global header that sets size, path or linkpath, which node-tar would apply to later entries, and its docstring no longer claims the two readers can never disagree. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the fail-closed checks and regression tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ot clean The wheel reader checks every entry against the bytes actually in the archive and caps the total on what was really inflated, not on the sizes the central directory claims. A bad record, an entry that does not fit, a size that does not match its data, encryption, ZIP64 or an unknown compression method now throws instead of ending the read early. Python packages gain the same unscanned state as npm ones: an artifact pinned by hash that does not unpack cleanly, a source with no files, a file list over the cap, or a code file over the size cap gives a high "could not be scanned" finding with unknown capabilities, listed in deps.unscanned rather than as not installed. AI-Assisted: Claude Code with Claude Opus 5.5; hardened the wheel reader, added the Python unscanned state and tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
The TOML reader caps array and inline-table nesting at 200 levels and throws a TOML error beyond it, and a lockfile that fails to parse is reported as "<file> could not be parsed: ..." instead of a stack overflow. AI-Assisted: Claude Code with Claude Opus 5.5; added the nesting cap, error handling and a regression test Co-Authored-By: Claude Opus 5.5 <[email protected]>
… and treats paths literally Every call through the hardened runner now also turns commit/tag signing off and overrides every gpg program and the ssh default-key command, reads no attributes from the work tree or the user's attributes file (--attr-source=<empty tree>, core.attributesFile=/dev/null), and empties each filter driver any config defines, so no clean, smudge or process filter runs. GIT_LITERAL_PATHSPECS=1 makes a path like pages/api/[id].js mean only that file. Subcommands that write (checkout, add, commit, reset) need git >= 2.40 and are refused while the repo's own config sets filter.*, gpg.*, include.* or includeIf.*. check-ignore and a paths-only reset join the allow-list for the loop's revert and ignored-path checks. AI-Assisted: Claude Code with Claude Opus 5.5; hardened the git runner and adapted the runner unit test Co-Authored-By: Claude Opus 5.5 <[email protected]>
… prompts show control characters applyDiff now rejects a diff that names a path twice or uses one path as both a file and a directory, puts back every file it already wrote when a later write fails, and writes each file to a temp file that is moved into place only if the target still holds the bytes it read (so a hard link is replaced, not written through, and the directory is re-checked to be inside the repo). Added lines with C0/C1 control characters (other than tab and a CRLF line ending) are refused, as are path segments containing ":" and the 8.3 alias of .gitmodules. The result carries each file's previous bytes and the hash of what was written, and restoreFiles() reverts from them. The plan printer shows control characters and bidi overrides as escapes in diffs, titles, paths and owner actions. The --fix-model context file must resolve inside the repo; package.json "main" pointing outside is ignored. AI-Assisted: Claude Code with Claude Opus 5.5; made applyDiff atomic and escaped the approval prompt, with tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
Dirty files are read from `status --porcelain=v1 -z` and re-checked right before each fix is written, so names with non-ASCII characters and files saved while the prompt was open are never patched. Fixes touching untracked or git-ignored paths are skipped. A revert restores each file's saved bytes only while it still holds what the fix wrote, tries every file, and reports what it left alone; a commit uses `--only` and happens only while the fix's files are unchanged. Anything edited meanwhile is left as is and the fix is reported as a conflict. AI-Assisted: Claude Code with Claude Opus 5.5; reworked the loop's revert/commit path and added regression tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
Writes no longer need git >= 2.40: emptying every configured filter driver, core.attributesFile=/dev/null and the refusal on repo-level filter.*/gpg.*/include.* keys already keep filters from running when the work tree's .gitattributes is read. --attr-source is still added on git >= 2.40. Below 2.31, where GIT_CONFIG_KEY_n is missing, writes are refused, and so are reads when a filter driver is configured. The repo config check runs before the version check, so the refusal names the config keys. AI-Assisted: Claude Code with Claude Opus 5.5; relaxed the git version gate and added old-git tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
…inks Evidence bundles are now built in a private temp folder next to the target and renamed into place, with every file created O_EXCL|O_NOFOLLOW. A caller can pin the real path the parent folder must keep resolving to (parentReal), which the dashboard uses for reports/evidence. verifyEvidence now lstats every path component of each artifact and of the manifest, reads through O_NOFOLLOW with a size cap (64 MB by default, maxBytes to change it), and treats manifest paths that differ only in case as duplicates. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the change and its regression tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
Session: the one-time token is printed in the URL fragment and redeemed through POST /api/session (Origin-checked, rate-limited), which returns the session id once. Every /api request must carry it as Authorization: Bearer; no cookie is set or accepted. The server still keeps only the SHA-256 of the id and compares in constant time. Static files are served without a session. The install smoke test now checks that /api/meta answers 401 without one. Approvals: a fix-loop approval must name the SHA-256 of the diff the page was given. Diffs over 60,000 characters are sent with truncated and fullLength, and approving one also needs acknowledgeTruncated. Answers are looked up with Object.hasOwn. Reports folder: a default reports/ that is a symlink is refused at startup (an explicit --reports may be one), and every write checks that the reports folder, and reports/evidence for bundles, still resolve to the paths checked at startup. Limits: event ids come from a per-run counter, so replay after the 5,000-event buffer is exact; finished runs are pruned to the last 20; open run streams are capped at 16; requests have a 30 s request timeout and a 2 min idle timeout, which the event stream turns off for its own socket; an oversized body gets a readable 413. --open hands the browser a 0600 launcher file that forwards to the URL, so the token is not on a command line. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the change and its regression tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
The page reads the one-time token from the URL fragment, removes it from the address bar, swaps it for a session id kept in sessionStorage and sends that as Authorization: Bearer on every request. The run transcript is read with fetch streaming (reconnecting with Last-Event-ID) instead of EventSource, and report downloads go through fetch, so both carry the header. The approval dialog sends the SHA-256 of the diff it rendered. A diff the server cut says so in the dialog, and Apply stays disabled (and does not take focus) until the user ticks that they have not seen all of it. AI-Assisted: Claude Code with Claude Opus 5.5; implemented the client side of the session and approval changes Co-Authored-By: Claude Opus 5.5 <[email protected]>
AI-Assisted: Claude Code with Claude Opus 5.5; updated the dashboard docs for the session and limits changes Co-Authored-By: Claude Opus 5.5 <[email protected]>
…e target port Response bodies are now read through readBody(): a streaming reader that counts decoded bytes (after Content-Encoding), stops at 5 MB by default, and honours the request timeout until the last byte instead of only until the headers arrive. Probes, the crawler, templates, recon and Retire.js fetches all go through it. Under --offline the egress check compares host and port (default ports on both sides still allow http -> https). Followed redirects drop Authorization, Cookie and Proxy-Authorization on any cross-origin hop, and a 307/308 that would replay a request body to another origin is returned to the caller instead of followed. AI-Assisted: Claude Code with Claude Opus 5.5; added readBody, routed callers through it, tightened egress and redirect handling, wrote tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
…er abort the audit The download-piped-to-shell, download-piped-to-interpreter, base64-piped-to-shell and PowerShell Invoke-Expression checks now find the same matches with linear-time helpers instead of regexes that rescan a line from every candidate start. The hook auto-approve regex, the pipe-to-shell preview and the Python version-pin check are rewritten to equivalent linear forms. canonicalJson is iterative, so a deeply nested MCP or hook definition is hashed instead of overflowing the stack, and each skill, instruction file, MCP server and hook is checked in isolation: one that throws is reported as "File could not be checked" and the audit continues. AI-Assisted: Claude Code with Claude Opus 5.5; rewrote quadratic agent-config matchers, made lock hashing iterative, isolated per-file checks, wrote CPU-time tests Co-Authored-By: Claude Opus 5.5 <[email protected]>
…treats $ as mandatory Inside any group repeated more than once, alternatives must start with disjoint characters, an optional item may not start like what follows it, and an iteration may not match the empty string. `$` now counts as a mandatory item in the rescan and overlap checks, so an unanchored unbounded run before `$` is refused. Bundled packs still pass rules:check unchanged. AI-Assisted: Claude Code with Claude Opus 5.5; extended the ReDoS guard's structural rules and its must-refuse corpus Co-Authored-By: Claude Opus 5.5 <[email protected]>
…uard Template matchers and extractors now run a pattern only if checkRegex accepts it (cached per pattern and flags), instead of the narrower nested-quantifier check. hasNestedQuantifier stays exported. AI-Assisted: Claude Code with Claude Opus 5.5; switched the template regex guard to checkRegex and added a regression test Co-Authored-By: Claude Opus 5.5 <[email protected]>
Drops the prepack script. pre-commit.com installs the node hook with `npm pack`, so every `pre-commit install-hooks` ran the whole suite. The publish workflow already runs `npm test` before publishing. AI-Assisted: Claude Code with Claude Opus 5.5; removed prepack and added a packaging assertion Co-Authored-By: Claude Opus 5.5 <[email protected]>
inside() now resolves the root itself and compares with relative(), so a root in git's forward-slash spelling on Windows compares equal even if a caller passes it unnormalised. gitRoot already returns a native path since 6cf41d6; this is the second layer, with a path.win32 unit test. AI-Assisted: Claude Code with Claude Opus 5.5; hardened inside() and added a path.win32 test Co-Authored-By: Claude Opus 5.5 <[email protected]>
…real pin in the example Both workflows check out with persist-credentials: false, so the token is not in .git/config while npm and the demo tapes run. demo-media sets up git auth through gh right before its push. The PR dependency-review example now pins the v1.6.1 release commit, the same pin `penthera init` writes, instead of an all-zero placeholder labelled v1.4.0. AI-Assisted: Claude Code with Claude Opus 5.5; tightened checkout credentials and fixed the example's action pin Co-Authored-By: Claude Opus 5.5 <[email protected]>
Packument, tarball and local-model responses are now read through readBody with a size cap and the request deadline, like every other caller. readBody gains a binary mode for tarballs. AI-Assisted: Claude Code with Claude Opus 5.5; routed the last two unbounded body reads through readBody and added the test Co-Authored-By: Claude Opus 5.5 <[email protected]>
AI-Assisted: Claude Code with Claude Opus 5.5; wrote the changelog Security section and documented the new hook option and loop behaviour Co-Authored-By: Claude Opus 5.5 <[email protected]>
AI-Assisted: Claude Code with Claude Opus 5.5; mapped the new finding category to its references Co-Authored-By: Claude Opus 5.5 <[email protected]>
…now reports AI-Assisted: Claude Code with Claude Opus 5.5; updated the audit baseline with reasons Co-Authored-By: Claude Opus 5.5 <[email protected]>
Dependency capability diffCompared against No package gained capabilities, and no new package can reach shell, network or install-time code. penthera deps v1.7.0: offline static analysis, no telemetry. Capabilities are what code can do, not a verdict. Checked by Penthera, open-source security for the apps and AI systems you ship · https://github.com/danoszz/penthera Updated for commit |
The before-upgrade phase runs 1.7.0, which prints ?token=; the new release prints #t=. Both answer 401 on /api/meta without a session. AI-Assisted: Claude Code with Claude Opus 5.5; fixed the smoke check for the old URL form Co-Authored-By: Claude Opus 5.5 <[email protected]>
When the repo's own git config sets filter.*, gpg.* or include.*, or git is older than 2.31, the loop stops before its first write with one message: the key(s) or version found, why, and the way forward (move the setting to global config and unset it here, or apply fixes by hand from `penthera fix --plan`). Scans and reports are unaffected. AI-Assisted: Claude Code with Claude Opus 5.5; reworded the refusal, added the test and doc lines Co-Authored-By: Claude Opus 5.5 <[email protected]>
danoszz
added a commit
that referenced
this pull request
Oct 20, 2026
… diff-bound approvals into the redesign main (#70, #71) moved the dashboard to a bearer session in the URL fragment and sessionStorage, a fetch-based run stream, SHA-256-bound approvals and cut-diff acknowledgement, in files the redesign had rewritten. Resolved by keeping the redesigned files and porting each change into them: - app.js: signIn() before the first API call. - center.js: the run stream uses api.stream; Apply (button, y, or y from the transcript) sends diffSha256 of the diff as shown; a cut diff shows the warning and needs the box ticked before Apply or y works. - panels.js, menubar.js: report downloads go through api.download, since a plain link cannot carry the session header (it now gets a 401). - right.js: session wording in Settings. - tests/ui.test.js: static files need no session now; the font and logo test checks type and content instead. - docs/dashboard.md: the cut-diff and SHA-256 rules move into the approval window paragraph; reopening a run points at the menu bar and History. - scripts/dashboard-screenshots.mjs: the #t= link, and globalThis for page globals (new lint step). AI-Assisted: Claude Code with Claude Opus 5.5; resolved the merge conflicts and ported main's session and approval changes into the redesign Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the findings of the second independent review: all highs and mediums, plus all of the lows except the few listed at the end. Most of these also affect 1.7.0, so this is written to ship as a patch release if the maintainer decides to. No version bump and no tag.
Every high and medium has a regression test that failed before the fix and passes after it. Hostile fixtures (archives, git configs, repos, servers) are built at runtime in temp dirs, so no exploit file is committed.
Findings → fix → test
unscanned, and install holds them. Caps are raised to the tarball reader's own (64 MB/file, 50k files) so real bundles liketypescript.js(9 MB) are scanned in fulldeps.test.js› "a code file too large to scan makes the package unscanned, not clean", "a package with more files than the source lists is unscanned, not clean", "scans a 9 MB index.js to the end…", "scans every file of a 5,001-file package…";install.test.js› "holds packages whose code is padded, spread over 5,001 files, or too large to scan"unscannedstate (high "could not be scanned" finding, held by install)deps-python.test.js› "a forged central-directory size makes the wheel unscanned…", "throws on structural anomalies rather than stopping early", "a source with an anomaly, no files, or more files than listed is unscanned", "scanDeps reports a pinned wheel that cannot be read…"core.attributesFile→ null device,--attr-source=<empty tree>(git ≥ 2.40), every configured filter driver emptied. Writes are refused if the repo's config setsfilter.*/gpg.*/include*, or on git < 2.31git-safety.test.js› "filters, signing and include.path in .git/config: reads neutralised, the loop refuses to write", "ssh signing's defaultKeyCommand…", "commit, add, checkout and status never run a filter or signer…", "git 2.31-2.39…" (×2), "refuses to write with a git too old…"status --porcelain=v1 -z; dirty state re-checked before each write and commit; revert restores pre-fix bytes only while the file still holds penthera's write (otherwiseconflict);commit --onlygit-safety.test.js› "(A) --allow-dirty with a dirty non-ASCII file…", "(B) …saved while the prompt was open…", "(C) …a failing fix is not reverted over it", "(C) …a passing fix is not committed with it"diff --cached --raw -z; includes type changes; scans files with NUL bytes; a file over 16 MB blocks (or warns withlargeFiles: "warn")hook.test.js› "staged secrets cannot slip past the hook" (5 cases), "a file over the size limit blocks by default and only warns when the policy says so"GIT_LITERAL_PATHSPECS=1git-safety.test.js› "a fix to pages/api/[id].js commits and reverts only that file…"git-safety.test.js› "never edits a git-ignored file…";fix.test.js› "restoreFiles restores what still holds the fix…"Authorization: Bearer; event stream read with fetch; no cookieui.test.js› "a valid session in a cookie is refused: only the Authorization header counts"reports/is refused at startup; every write re-checks that the folder still resolves to itui.test.js› "refuses a default reports folder that is a symlink…", "refuses every write once reports/ … no longer resolves…"truncated/fullLength; approval needs the sha256 of the diff shown, plus an acknowledgement when cut; the client shows a "Diff cut" markerui.test.js› "a 70,000-character diff says it is cut, and cannot be approved without the matching hash and an acknowledgement"applyDiff(and so the model dry run)fix.test.js› "refuses added lines with control characters…", "shows control characters as escapes in diffs, titles and paths"ai-secrets.test.js› "a placeholder does not hide a real secret of the same rule later in the text";hook.test.js› "a placeholder of the same rule earlier in the file"readBodywith a decoded-size cap and a deadline covering the body; every body reader goes through it, including registry, tarball and local-model readshttp-limits.test.js› "an endless body ends at the request timeout", "a gzip bomb stops at the decoded size cap", …;fix.test.js› "stops reading a model response that never ends"agents-linear.test.js› "1 MB of "curl "…", "reports the same match text and line as before"$counts as mandatoryrule-packs.test.js› "rejects /…/" for 8 new must-refuse patternsinsidenow resolves the root and compares withrelative(); unit-tested withpath.win32onboarding.test.js› "accepts a Windows root in git's forward-slash spelling, refuses escapes"prepackremoved (publish.yml still runsnpm test)packaging.test.js› "package.json has no pack or install lifecycle scripts"applyDiffis all-or-nothing: duplicate and prefix paths refused, rollback on errorfix.test.js› "refuses a diff that writes one path twice…", "puts back every file it wrote when a later write fails"nextSeqfor event idsui.test.js› "event ids keep counting past the 5,000-event buffer…"fix.test.js› "replaces a hard-linked file instead of writing through it":andgitmod~Nrefusedfix.test.js› "refuses NTFS alternate data stream names and the 8.3 alias of .gitmodules"grecords that set size/path/linkpath refuseddeps.test.js› "refuses a pax global header that sets size, path or linkpath"O_EXCL|O_NOFOLLOW; verify lstats every component, caps reads, dedupes case-insensitivelyevidence.test.js› "never reads through a symlinked folder…", "caps how much it reads…", "builds the bundle in a private temp folder…"canonicalJson; per-file errors become findings; TOML depth capai-secrets.test.js› "does not read a FIFO named .env…";agents-linear.test.js› "penthera agents completes on a 200,000-deep .mcp.json…";deps-python.test.js› "a deeply nested lockfile is a parse error…"checkRegexcode-scanning.test.js› "refuses alternation and rescanning shapes too…"hook installlstats, treats a symlink as foreign (backs it up), writes withwxhook.test.js› "moves a symlinked pre-commit aside instead of writing through it"--offlinecompares host and port; cross-origin hops drop credentials and don't replay bodieshttp-limits.test.js› "compares the port…", "drops Authorization and Cookie on a cross-origin hop", "does not replay a 307 body…"Object.hasOwn; (c) request/idle timeouts, finished-run pruning, stream cap; (d) 413; (e)--openuses a 0600 launcher file instead of a token on the command line. (b) is in PR #63's client, see belowui.test.js› "prototype keys are not answers", "bounds request time and idle sockets…", "keeps a bounded number of finished runs", "caps open run streams", "answers 413…", "writes a launcher page only this user can read…"fixHintand agents rules (AUTO_APPROVE, preview, Python pin)hook.test.js› "fixHint stays linear…";agents-linear.test.js› 3 casespersist-credentials: false; real pin in the example workflowfix.test.js› "never sends a file outside the repo, via package.json main or a symlink"Lows not done, with reasons
init < /dev/nullshould install the hook), not a bug fix. It needs the maintainer.node_modulesand Windowsshell: true: both unverified; they need a Windows or planted-tree repro first.npm ciin the id-token job andinstall.shfrom unpinned main: release-process changes, so they belong with the release work.--baselinegates on upgrade. The match-by-match fix covers the scanning side.Behaviour changes to know about
.git/configsetsgpg.*,filter.*orinclude.*, which includes legitimate per-repo signing setups. Global config is unaffected."largeFiles": "warn"to relax). The old cap was a silent 2 MB./apistill does. Server (1d7ba83) and client (afcf4ad) must land together.audit/baseline.jsonwith reasons.PR #63
PR #63 rewrites the dashboard client. The server, evidence and test commits cherry-pick onto it, apart from one test about serving static files without a session. The client commit does not. An equivalent client change for #63's files, including L11b (Apply no longer auto-focused, no global
ykey), is on branchsec2/ui-for-pr63. Whichever of the two merges second needs that commit.Checks
npm test: 1,010 passed, 21 skipped.npm run audit:internal: all gates pass (packed 667 KB, unpacked 1,772 KB, 185 files).🤖 Generated with Claude Code