Skip to content

Integrate vetted fixes from open upstream PRs (page 1) - #1

Merged
adri22235 merged 27 commits into
mainfrom
integration/page1-fixes
Oct 2, 2026
Merged

adri22235 merged 27 commits into
mainfrom
integration/page1-fixes

Conversation

@adri22235

Copy link
Copy Markdown
Owner

Summary

Applies the sound fixes from the first page of open pull requests in anthropics/claude-code onto this fork. Original authorship is preserved on every commit.

Applied (12 commits)

Not applied

Test plan

  • plugins/security-guidance/tests: 37 unit tests pass
  • test-sg-python.sh: 24 checks pass
  • validate-agent.test.sh: 5 checks pass
  • test_extensibility.py: 17 tests pass
  • JSON files, pr-review-toolkit YAML frontmatter and shell syntax validated
  • No tests exist for the hookify, plugin-dev (except validate-agent) and pr-review-toolkit changes

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoLAv3aLaRJocsRMcjFbpv


Generated by Claude Code

genesisdayabl-droid and others added 25 commits October 2, 2026 15:30
…ample.py

The example script pointed to the old docs.anthropic.com domain while
every other reference in the repo uses code.claude.com/docs/en/...
Eight bundled skills declare a title-cased `name` with spaces, e.g.

    name: Skill Development     # plugins/plugin-dev/skills/skill-development/
    name: MCP Integration       # plugins/plugin-dev/skills/mcp-integration/

The Agent Skills specification requires the `name` field to contain only
lowercase alphanumerics and hyphens, and to match the parent directory
name. Both constraints are violated by all eight, so `skills-ref validate`
rejects them.

The two skills outside plugin-dev/hookify already follow the spec —
`claude-opus-4-5-migration` and `frontend-design` — so this changes the
eight outliers to the convention the repository already uses elsewhere.

Only the frontmatter `name` field is touched. The prose headings in
plugins/plugin-dev/README.md and the `#` titles inside each SKILL.md keep
their human-readable capitalization, since neither is a skill identifier.

Spec: https://agentskills.io/specification#name-field
…) and stop false-flagging valid agents

Two defects made the validator fail on plugin-dev's own agent files
(anthropics#83803):

1. Under `set -e`, `((warning_count++))` / `((error_count++))` return a
   nonzero status when the counter was 0, so the script died at the first
   warning or error instead of finishing the run. Increments now use
   `count=$((count + 1))`, which always returns 0.

2. Field extractions like `TOOLS=$(... | grep '^tools:' ...)` aborted the
   script under `set -e` when the field was absent (grep exits 1 on no
   match), instead of reporting the missing field. They now end in
   `|| true`.

3. The description check only read the first physical line of the
   `description:` value, so multi-line descriptions with <example> blocks
   (as in plugin-dev's own agents) were false-flagged as missing examples.
   The extraction now captures the full multi-line value.

Adds validate-agent.test.sh: plugin-dev's own agents must exit 0, a
warning-only file must complete with exit 0, and an invalid file must
still exit 1 with all errors reported.

No-Verification-Needed: standalone shell script in the public repo; driven end-to-end directly plus new regression harness
Reuse the existing _DOC_EXTS path filter for the four XSS-family substring rules so documentation examples do not emit security warnings.

Add regression coverage for every documentation extension while keeping the existing executable-source detections intact.
Every agent's description was a single unquoted scalar containing
dialogue lines like Daisy: "..." / Assistant: "...", which YAML
parses as an illegal nested mapping. That leaves the agent loading
with empty frontmatter (name/description/model/color all silently
dropped), so it can't be selected by description-based routing.

silent-failure-hunter.md was reported (anthropics#86748); the same defect was
present in the other five agents in this plugin, so all six are
converted to a `description: |` block scalar. Body content and
description text are unchanged, only the frontmatter encoding.

Fixes anthropics#86748

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…h with a quoted path

The Stop hook in ralph-wiggum and the SessionStart hooks in
learning-output-style and explanatory-output-style gave a bare, unquoted
"${CLAUDE_PLUGIN_ROOT}/....sh" as the hook command. Shell-form hook
commands are expanded by the shell from the CLAUDE_PLUGIN_ROOT env var,
so a plugin root containing a space word-splits and the hook exits 127
("not found") with no visible effect; the bare form also relies on the
script's exec bit and, on Windows, on the CLI prepending an interpreter.

Invoke the scripts as `bash "${CLAUDE_PLUGIN_ROOT}/....sh"`, the form
security-guidance already uses, and bump the three plugins to 1.0.1 so
existing installs pick the change up on update (the plugin.json version
is what the updater compares).

Refs anthropics#95673, anthropics#78490.
…es in plugin scripts

Applied from anthropics#84711 without the ralph-wiggum stop-hook.sh
change (rejecting any transcript path containing '..' or that is a symlink
would end the loop for legitimate setups).
… reach; mask credential values in review feedback and session state

Applied from anthropics#96434 (both commits, squashed). Unit tests
in plugins/security-guidance/tests pass.
…not twice

sg-python.sh probed each candidate and then exec'd the chosen one, so every
hook cost at least two interpreter launches. On Windows with the Python
Install Manager, each launch of the WindowsApps alias starts an AppX update
that leaks ~1.6 MB in AppXSvc, which exhausted system commit in the report.

Cache the name of the candidate that passed the probe (python3, python or
py -3 only; never run arbitrary file contents) in ~/.claude/security/python-cmd.
Later hooks exec it directly with a single launch. The entry expires after a
day and is ignored when the command is no longer on PATH. Cache failures never
block the hook.

Not addressed here: preferring real interpreter paths over WindowsApps aliases.

Addresses upstream issue 98929 (Windows: py -3 launched twice per hook).

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01LoLAv3aLaRJocsRMcjFbpv
…sing

Line 31 runs a gcloud command substitution under set -euo pipefail. Without
gcloud on PATH it exits 127 and takes the script with it, so the PROJECT_ID
check on line 80 never runs and the user gets no output at all. The AWS
example already guards the same construct with || true.
${DIST_SHA256,,} is a bash 4 case-modification expansion. macOS ships bash
3.2 as /bin/bash, so the line aborts the script with "bad substitution"
before it reaches its own argument checks. Replace it with printf | tr,
pinned to LC_ALL=C so BSD tr cannot fail the script on a stray non-UTF-8
byte under set -e.
The Read tool silently fails on PDFs without poppler-utils, which is
currently undocumented and missing from default container setups.
This PR adds poppler-utils to the .devcontainer provisioning to
ensure PDF rendering works out-of-the-box in containerized environments.

Closes anthropics#23704
…ndbox unavailable

The settings-bash-sandbox.json example is documented as 'Bash tool must
run inside of sandbox', but without failIfUnavailable Claude Code falls
back to running commands unsandboxed (with a warning) whenever the
sandbox cannot initialize - including on all native Windows hosts and
Linux hosts without bubblewrap. Add failIfUnavailable: true to match the
managed-enforcement configuration recommended in the sandboxing docs,
and document the platform constraint in the README.

Co-Authored-By: Claude Fable 5 <[email protected]>
Both reads used GitHub's default page size of 30 and never followed
pagination, unlike the issues list in the same script and every list
call in sweep.ts. A thumbs-down past the first 30 reactions, or human
replies after a dupe notice sitting at comment position 30, were
invisible to the guards, so issues auto-closed despite objections.

Add a githubRequestAllPages helper (per_page=100, follows pages until a
short page) and use it for both reads.

Fixes anthropics#80506
init-firewall.sh only configured iptables, so on dual-stack Docker
networks all IPv6 traffic bypassed the allowlist entirely - a hole in a
script whose documented purpose is default-deny egress for running
Claude Code with reduced supervision.

This mirrors the IPv4 posture for IPv6:
- default-DROP policies with loopback, DNS, SSH-response, link-local,
  and ICMPv6 (neighbor discovery) allowances
- an allowed-domains-v6 ipset (family inet6) populated from GitHub
  meta's IPv6 ranges and AAAA records of the existing allowlist domains
- fast REJECT (icmp6-adm-prohibited) for everything else so blocked
  IPv6 attempts fall back to IPv4 instead of hanging
- a curl -6 negative verification alongside the existing checks

All ip6tables usage is gated behind a capability probe: on hosts with
IPv6 disabled in the kernel, ip6tables cannot operate and there is no
IPv6 traffic to filter, so the script skips IPv6 rules with a warning
instead of hard-failing container startup under set -e.

Co-Authored-By: Claude Fable 5 <[email protected]>
The /ralph-loop command substituted $ARGUMENTS directly into the
auto-executed shell line, so the user's prompt text was parsed as shell
code. Any prompt containing an apostrophe (Bob's), a semicolon, $(...),
or a newline made the setup command fail its permission check (or fail
to parse) before the loop ever started:

  Error: Shell command permission check failed for pattern ...
  This Bash command contains multiple operations.

With permission checks bypassed, $(...) in a prompt would even execute.

Feed $ARGUMENTS to setup-ralph-loop.sh via a quoted heredoc on stdin
instead, so prompt text is never shell-parsed, and parse the
--max-iterations / --completion-promise options textually in the
script. Direct argv invocation still works. The state file format is
unchanged, so stop-hook.sh is unaffected.

Fixes anthropics#16037
… fails to resolve

init-firewall.sh runs under `set -e` and exits 1 as soon as any domain in the
allowlist fails to resolve. Since statsig.anthropic.com stopped resolving, that
single NXDOMAIN aborts the whole script: the ipset is left half-populated, the
default DROP policies are never applied, and the devcontainer fails to start
with exit code 1.

A telemetry or marketplace endpoint disappearing should not be able to break
container startup. The domain list is split into required domains, which still
fail loudly because the container is useless without them, and optional ones,
which are skipped with a warning and summarized at the end so the reason for a
later connection failure stays visible.

Invalid (non-IPv4) DNS answers get the same treatment: hard error for required
domains, skip with a warning otherwise.

Verified with a stubbed `dig`/`ipset`:

  statsig.anthropic.com unresolvable
    before: "ERROR: Failed to resolve statsig.anthropic.com", exit 1
    after:  warning, remaining domains still added, exit 0
  api.anthropic.com unresolvable (required)
    after:  "ERROR: Failed to resolve required domain api.anthropic.com", exit 1
  all domains resolvable
    after:  unchanged behaviour, exit 0

shellcheck clean.

Fixes anthropics#55623

Merged onto the IPv6 change (upstream PR 81423) and current main: the domain
lists follow main, which no longer allowlists statsig.anthropic.com, and the
AAAA lookup applies the same rule as the A lookup (a dig failure or a bad
address skips an optional domain instead of aborting).
@adri22235
adri22235 marked this pull request as ready for review October 2, 2026 16:14
fix(security-guidance): launch Python once per hook, not twice
Integrate vetted fixes from open upstream PRs (page 2)
@adri22235
adri22235 merged commit 5ab4e7f into main Oct 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.