Fix tool denials and restore build validation in release docs workflow - #1114
Conversation
The v0.44.0 run (128 turns, $10.68) hit 19 tool denials. Four causes,
all fixable; one was a real bug rather than noise.
Prefix-match the npm and npx allowlist entries. `Bash(npm run
prettier:fix)` matches only the bare command, so the skill's attempts
at `npm run prettier:fix -- <files>` and `npx prettier --write
<files>` were both denied. That made the prompt's own instruction
("format your edits in this session; CI lint does not run on commits
pushed by this bot") unfollowable. The workflow's auto-fix step was
covering for it. `Bash(npm run:*)` is not a wider trust boundary:
every script comes from the repo's package.json, which later steps in
this job already run.
Hand the skill the commit list. Five denials were repeated attempts at
`git log v0.43.0..v0.44.0` against the upstream clone, which cannot
succeed: Claude Code refuses `git -C <path>` because it can execute
untrusted hooks, and the clone lives outside the session's working
directory. `.release-meta.json` now carries a `commits` array, built
from the compare response the step already fetches for contributors.
Also add read-only git entries (log, diff, show, tag, check-ignore)
for use inside the workspace.
Tell both prompts how to read the clone. Read, Grep, and Glob work
against it and were already doing the real source verification (15
successful calls, zero fallbacks to `gh api contents`), so the Bash
attempts were wasted turns rather than degraded output. Also state
that appending `2>&1` or chaining with `&&` splits a command into
parts the allowlist no longer matches.
Install the ToolHive CLI so `npm run build` works. The
mcp-metadata-plugin shells out to `thv registry info` at build time,
and docusaurus.config.ts sets its failOnError whenever CI=true, so
the skill's Phase 5 build validation has never actually run in this
workflow. That is the step that catches broken links and bad
references in generated pages. Uses the same script Vercel's
buildCommand runs. Verified locally: with thv present and CI=true the
build completes and the plugin resolves metadata for 9 servers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates release documentation automation to reduce tool denials, pass commit metadata, and restore ToolHive-backed build validation.
Changes:
- Installs ToolHive CLI before documentation validation.
- Adds release commit metadata to
.release-meta.json. - Updates prompts, clone guidance, and Claude command permissions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Summary | Final review comments |
|---|---|---|
.github/workflows/upstream-release-docs.yml |
Updates workflow setup, metadata generation, prompts, and permissions. | Critical: Narrow overly broad command allowlists and make Git tag operations read-only (4 votes). Critical: Pin and verify the ToolHive asset (1 vote). Moderate: Persist fallback CLI installation paths and handle truncated commit comparisons (3 votes each). |
.claude/skills/upstream-release-docs/SKILL.md |
Documents clone usage and the commit metadata contract. | Nit: Update source-verification instructions to prefer the provided clone and reserve gh api for runs without one (2 votes). |
Suppressed comments (3)
.github/workflows/upstream-release-docs.yml:865
- This changes the Claude sandbox from a small set of known scripts to arbitrary
npm runscripts, andgit tag:*is not read-only (git tag -d/-fcan mutate refs). Because this job checks out PR-controlled files and hascontents:write, a prompt-injected or compromised input can use these prefixes to execute unintended repository code or alter refs. Keep the allowlist to the required commands and argument forms, for example the exact build/check scripts plusnpm run prettier:fix:*andnpm run eslint:fix:*; use a read-onlygit tag --listform or remove it.
--allowedTools "Bash(gh:*),Bash(npm run:*),Bash(npx prettier:*),Bash(npx eslint:*),Bash(git log:*),Bash(git diff:*),Bash(git show:*),Bash(git tag:*),Bash(git check-ignore:*)"
.github/workflows/upstream-release-docs.yml:570
- On a failed compare, this writes
commits: [], but the new skill contract treats any suppliedcommitsarray as authoritative and only derives the range when the handoff file is absent. Sincecompare_okis not included in.release-meta.json, a transient API failure is indistinguishable from a genuinely empty release and can silently prevent commit-based classification and reviewer requests. Include the compare status in the handoff and make the skill fall back to its release-notes/API path when the compare failed.
COMMITS_JSON='[]'
if [ -n "$COMPARE_JSON" ]; then
COMMITS_JSON=$(printf '%s' "$COMPARE_JSON" | jq -c '
[ .commits[]
| { sha: (.sha[0:8]),
subject: (.commit.message | split("\n")[0]),
author: (.author.login? // .commit.author.name? // null) } ]' \
2>/dev/null || echo '[]')
fi
.github/workflows/upstream-release-docs.yml:568
.commit.author.nameis a display name, not a GitHub login, so this fallback violates theauthor logincontract stated immediately above and inSKILL.md. For an unlinked or deleted account, the skill can mistake a name for a contributor when grouping commits; emitnullfor an unknown login or add a separateauthor_namefield.
author: (.author.login? // .commit.author.name? // null) } ]' \
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # buildCommand runs. Required, not soft-failed, so a broken | ||
| # install fails here rather than mid-skill. | ||
| - name: Install ToolHive CLI (for build-time MCP metadata) | ||
| run: ./scripts/install-thv.sh |
There was a problem hiding this comment.
Not taking this one as proposed. The concern is legitimate; the suggested remedies mostly aren't.
Checksum verification buys close to nothing here. toolhive does publish toolhive_<version>_checksums.txt, so it's possible. But the checksum would come from the same API response that supplies the tarball URL. An attacker who can replace a release asset can replace the checksums file in the same breath, so this defends only against a MITM on a download that is already HTTPS. It reads as a control without being one.
Pinning to "the release being processed" is wrong. This workflow also runs for toolhive-studio and toolhive-registry-server. Those tags have no thv asset, so pinning the CLI to the release under documentation would break the step for two of three projects.
Signature verification is the real version of this, and it's viable: *.sigstore.json bundles are published per artifact. That needs cosign plus an identity policy, which is its own change with its own failure modes, not a line edit inside this PR. Happy to file it as a hardening issue.
One correction to the framing: install-thv.sh is not newly trusted. Vercel's buildCommand already runs the same script on every production build, so the repo has been executing this binary from releases/latest for a while. Your sharper point stands, though, and it's the part I'd want captured in that issue: this job holds contents: write and pushes commits, which the Vercel build does not, so the same script carries more consequence here.
There was a problem hiding this comment.
Filed as #1115 with the sigstore approach, and the two constraints noted there: don't pin the CLI to the release being documented, and verification needs an identity policy rather than a bare verify-blob. Leaving this thread open until that lands.
Tighten the allowlist instead of widening it. The npm scripts are `prettier . --write` and `eslint . --fix`, so they already act on the whole repo and never needed the file list the skill was passing. Both prompts now say to run them with no arguments, which removes the denials without the prefix-match entries, so the exact-match forms are back. Dropped `Bash(git tag:*)`: `git tag -d/-f` mutates refs, and the commit list arriving as data removes the reason the skill wanted it. Assert thv is on PATH. install-thv.sh falls back to ~/.local/bin when /usr/local/bin isn't writable and exports PATH only in its own shell, which later steps would not see. Persist it via GITHUB_PATH and fail in this step rather than as a confusing build error mid-skill. Detect compare truncation. The endpoint caps at 250 commits, past which both the commit list and the contributor list are silently partial. Emit a warning and a `commits_truncated` flag, and tell the skill to fall back to the release notes and record it in SUMMARY.md. Verified: v0.33.0..v0.44.0 returns 250 of 307 and trips the flag; normal single-release ranges do not. Resolve a contradiction in the skill. Core Principle and Phase 2 step 6 told the agent to verify source via `gh api contents` unconditionally, which conflicted with the new guidance to read the provided clone. Both now branch on whether the caller supplied a clone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Description
The v0.44.0 run (#1113) recorded 19 tool denials. This fixes all four causes.
Prefix-match the npm and npx allowlist entries.
Bash(npm run prettier:fix)is an exact match, so any argument breaks it andnpm run prettier:fix -- <files>was denied. That made the prompt's own instruction unfollowable: "Do not defer formatting to CI... Format your edits in this session." It couldn't; the workflow's auto-fix step was covering for it. The prefix form isn't a wider trust boundary, since every script comes from the repo'spackage.json, which later steps in this job already run.Pass the commit list in
.release-meta.json. Five denials were repeated attempts atgit log v0.43.0..v0.44.0against the upstream clone, which can never succeed:git -C <path>is refused outright (it can execute untrusted hooks) and the clone is outside the session's working directory. The step now emits acommitsarray from the compare response it already fetches, so no extra API call.Tell both prompts how to read the clone. Read, Grep, and Glob work against it and were already doing all the real source verification, so this was wasted turns rather than degraded output. Also: appending
2>&1or chaining with&&splits a command into parts the allowlist no longer matches.Install the ToolHive CLI so
npm run buildworks. Separate finding from the same run.plugins/mcp-metadata-pluginshells out tothv registry infoat build time anddocusaurus.config.tssets itsfailOnErrorwheneverCI === 'true', so the skill's Phase 5 build validation has never actually run in this workflow. That's the step that catches broken links and bad references in generated pages. Usesscripts/install-thv.sh, the same script Vercel'sbuildCommandruns.Type of change
Related issues/PRs
Follow-up to #1111, from denials observed in the v0.44.0 run on #1113. No overlap with #1112.
Submitter checklist
Content and formatting
Reviewer checklist
Content
Testing
.release-meta.jsongenerated against the real v0.43.0 to v0.44.0 range: 4 contributors (matching what the live run classified) and 8 commits. Failed-compare path still yields an empty list and exit 0.npm run buildwithCI=trueandthvpresent: completes, plugin resolves metadata for 9 MCP servers. This is the case that was failing in CI.install-thv.shresolves (toolhive_0.44.0_linux_amd64.tar.gz) exists on the latest release.shellcheckclean across all 42runblocks and oninstall-thv.sh; Prettier clean;SKILL.mdanchor links resolve.The denial count on the next release run is the real check.