feat: capability-based default-branch protection with local fallback - #2
Conversation
Phase 2 required a GitHub ruleset on main. GitHub reserves that for paid plans on private personal repositories and answers "Upgrade to GitHub Pro or make this repository public to enable this feature", so a solo developer on a free plan could only pass the bootstrap gate by paying or by publishing a private repository. Neither is an acceptable price for a lifecycle gate. The requirement is now stated behaviourally and satisfied at the strongest tier the host and account actually grant: Tier 1, server side. GitHub rulesets, falling back to classic branch protection on older Enterprise Server, and GitLab protected branches. Self-hosted and unrecognised hosts skip straight to tier 2. Tier 2, managed local. A pre-push guard reading git's ref-update records from stdin, refusing deletion of the protected branch and non-fast-forward updates to it, allowing fast-forward pushes and initial creation, and failing closed when it cannot see the records. The gate item "default-branch history protection verified" accepts either, tier 2 only with its narrower trust boundary recorded. Repository visibility is never changed: every provider call goes through one choke point that refuses to issue a visibility mutation at all. Verification runs against disposable repositories under the system temp directory, never a real remote, and every recursive delete is refused unless the path is a directory the tool itself created with its own prefix. Existing projects blocked on a paid-plan ruleset resume with `branch-protection.js migrate`, which re-detects capability, stands up the fallback, and names exactly which recorded blocker to clear. Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request adds provider-aware default-branch protection, a managed local history guard, hook wiring, verification, migration, state reporting, comprehensive Node tests, CI coverage, and version 1.1.0 documentation. ChangesBranch protection workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ForgeWorkflow
participant BranchProtection
participant ProviderAPI
participant HistoryGuard
ForgeWorkflow->>BranchProtection: detect and apply protection
BranchProtection->>ProviderAPI: create or update server-side rules
ProviderAPI-->>BranchProtection: return verification data
BranchProtection->>HistoryGuard: install local fallback when required
HistoryGuard-->>ForgeWorkflow: reject deletion or non-fast-forward updates
BranchProtection-->>ForgeWorkflow: report tier, evidence, and gate status
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
CHANGELOG.md (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHyphenate the compound modifier.
Use
capability-basedbefore the noun at both sites.
CHANGELOG.md#L45-L45: Change "capability based" to "capability-based".README.md#L80-L80: Change "capability based" to "capability-based".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 45, Hyphenate the compound modifier at both affected sites: change “capability based” to “capability-based” in CHANGELOG.md line 45 and README.md line 80.Source: Linters/SAST tools
CONTRIBUTING.md (1)
9-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare the fenced-code languages.
Markdownlint reports MD040 for these three fences. Use
shfor the shell commands. Usetextfor/reload-plugins.Also applies to: 34-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 9 - 19, Declare languages on all three fenced code blocks in CONTRIBUTING.md: use sh for the shell command examples and text for the /reload-plugins block, including the additional fence referenced by the review.Source: Linters/SAST tools
templates/branch-protection.js (1)
1321-1381: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the read-back detail when a remote apply succeeds but does not verify.
If
applied.appliedis true andverified.verifiedis false,applied.failureis undefined andattempts[0].failureis null.remoteFailurethen falls back to the generic text "server-side protection could not be established". The state file loses the reason the host actually gave, which is the most useful part of the record.Carry
verified.detailinto the fallback reason.♻️ Proposed change
let applied = null; + let verifyDetail = null; if (capability.serverSide !== "no") { applied = adapter.apply(capability); @@ const verified = adapter.verify(capability, applied); attempts[attempts.length - 1].verified = verified.verified; attempts[attempts.length - 1].detail = verified.detail; + if (!verified.verified) verifyDetail = verified.detail; @@ const remoteFailure = (applied && applied.failure) || (attempts[0] && attempts[0].failure) || { - kind: "unknown", - message: "server-side protection could not be established", + kind: verifyDetail ? "unverified" : "unknown", + message: + verifyDetail || + "server-side protection could not be established", };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/branch-protection.js` around lines 1321 - 1381, Update the remote failure handling after the verification branch so a successful apply with failed verification uses verified.detail as the fallback reason. Preserve the existing applied.failure and attempts[0].failure precedence, and only use the generic message when no read-back detail is available. Anchor the change to the remoteFailure construction and the verified result from adapter.verify.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/branch-protection.js`:
- Around line 266-294: Update the command parsing logic around current and the
direct-child check to record the first indented child level encountered for each
command, rather than requiring commandIndent + 2. Use that recorded child indent
for subsequent run, use_stdin, priority, and conditional fields, while
preserving isolation between nested mappings and resetting the tracked indent
for each new command.
- Around line 679-683: Update the ruleset lookup in the existing `ghJson` call
to combine all paginated response pages into a single JSON document before
parsing, using `--slurp` only if supported by the repository’s minimum `gh`
version; otherwise remove pagination or implement an equivalent aggregation.
Preserve the existing lookup behavior so `apply` reliably detects existing
rulesets.
In `@templates/history-guard.js`:
- Around line 212-221: Update the main failure-message handling for the result
returned by readStdin so reason "unreadable" prints the underlying error from
its error property instead of MISCONFIGURED_HELP; preserve the existing message
for genuinely empty stdin and other failure reasons.
In `@tests/repo-standards.test.js`:
- Around line 78-110: Update the repository traversal test around roots,
explicit root files, and the extension filter so it covers .claude-plugin and
docs directories, the root package.json, and HTML files. Ensure both tracked and
untracked working-tree files in these locations are scanned for forbidden
characters, while preserving the existing violation reporting and assertion
behavior.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 45: Hyphenate the compound modifier at both affected sites: change
“capability based” to “capability-based” in CHANGELOG.md line 45 and README.md
line 80.
In `@CONTRIBUTING.md`:
- Around line 9-19: Declare languages on all three fenced code blocks in
CONTRIBUTING.md: use sh for the shell command examples and text for the
/reload-plugins block, including the additional fence referenced by the review.
In `@templates/branch-protection.js`:
- Around line 1321-1381: Update the remote failure handling after the
verification branch so a successful apply with failed verification uses
verified.detail as the fallback reason. Preserve the existing applied.failure
and attempts[0].failure precedence, and only use the generic message when no
read-back detail is available. Anchor the change to the remoteFailure
construction and the verified result from adapter.verify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45af6f10-39af-4ebf-8cf7-baf56d5d1fe3
📒 Files selected for processing (21)
.claude-plugin/plugin.json.github/workflows/validate.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/index.htmlpackage.jsonskills/forge-env/SKILL.mdskills/forge-standards/SKILL.mdskills/forge/SKILL.mdtemplates/branch-protection.jstemplates/history-guard.jstemplates/lefthook.ymltests/disposable-remote.test.jstests/gate-and-migration.test.jstests/helpers/sandbox.jstests/history-guard.test.jstests/hook-wiring.test.jstests/protection-capability.test.jstests/repo-standards.test.js
| if (commandIndent === null) commandIndent = indent; | ||
|
|
||
| if (indent === commandIndent && /^[A-Za-z0-9_.-]+\s*:\s*$/.test(body)) { | ||
| current = { | ||
| key: body.replace(/\s*:\s*$/, ""), | ||
| run: "", | ||
| useStdin: false, | ||
| priority: null, | ||
| conditional: false, | ||
| }; | ||
| commands.push(current); | ||
| continue; | ||
| } | ||
| if (current === null) continue; | ||
|
|
||
| // Only direct children of the command key. A nested mapping such as | ||
| // skip: with its own run: must not overwrite the command's own fields. | ||
| if (indent !== commandIndent + 2) continue; | ||
|
|
||
| if (/^run\s*:/.test(body)) { | ||
| current.run = body.replace(/^run\s*:\s*/, ""); | ||
| } else if (/^use_stdin\s*:/.test(body)) { | ||
| current.useStdin = /true/i.test(body); | ||
| } else if (/^priority\s*:/.test(body)) { | ||
| const n = parseInt(body.replace(/^priority\s*:\s*/, ""), 10); | ||
| current.priority = isNaN(n) ? null : n; | ||
| } else if (/^(skip|only)\s*:/.test(body)) { | ||
| current.conditional = true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not hardcode a two-space child indent for lefthook commands.
Line 283 accepts only indent === commandIndent + 2 as a direct child of the command key. A lefthook.yml written with four-space indentation has its run: and use_stdin: lines skipped. checkLefthookWiring then reports found: false, installLocal refuses to proceed, and the gate fails on a correctly wired project.
Track the first child indent per command instead of assuming a fixed step.
🐛 Proposed fix: derive the child indent from the file
if (indent === commandIndent && /^[A-Za-z0-9_.-]+\s*:\s*$/.test(body)) {
current = {
key: body.replace(/\s*:\s*$/, ""),
run: "",
useStdin: false,
priority: null,
conditional: false,
+ childIndent: null,
};
commands.push(current);
continue;
}
if (current === null) continue;
// Only direct children of the command key. A nested mapping such as
// skip: with its own run: must not overwrite the command's own fields.
- if (indent !== commandIndent + 2) continue;
+ if (current.childIndent === null) current.childIndent = indent;
+ if (indent !== current.childIndent) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/branch-protection.js` around lines 266 - 294, Update the command
parsing logic around current and the direct-child check to record the first
indented child level encountered for each command, rather than requiring
commandIndent + 2. Use that recorded child indent for subsequent run, use_stdin,
priority, and conditional fields, while preserving isolation between nested
mappings and resetting the tracked indent for each new command.
| const existing = ghJson([ | ||
| "api", | ||
| "--paginate", | ||
| "repos/" + slug + "/rulesets?includes_parents=false", | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
gh api --paginate --slurp multiple JSON arrays output version
💡 Result:
When using the GitHub CLI command gh api with the --paginate flag, the default behavior is to output each page of results individually, which can result in multiple separate JSON objects or arrays being printed to the output stream [1][2]. By adding the --slurp flag, gh api will capture all these individual pages and wrap them into a single, top-level JSON array [1][3]. In this combined structure, each element of the top-level array corresponds to the full JSON response (either an object or an array) returned by an individual API page request [4]. Key details regarding this functionality include: - Requirement: The --slurp flag can only be used in conjunction with the --paginate flag [1][2]. - Compatibility: You cannot use --slurp at the same time as --jq or --template [2]. This is because the CLI buffers the response pages in memory to construct the final array, which is mutually exclusive with those specific output-processing flags [2]. - Use Case: This feature is primarily useful when you need the complete set of paginated results to be valid JSON, such as for further processing by tools like ConvertFrom-Json in PowerShell or other JSON-parsing utilities that expect a single, unified document [5][4]. - GraphQL Considerations: For GraphQL requests, ensure your query is set up to handle pagination via the pageInfo object (using hasNextPage and endCursor) to allow the CLI to fetch all necessary pages before they are slurped together [1][3].
Citations:
- 1: https://cli.github.com/manual/gh_api
- 2: https://github.com/cli/cli/blob/trunk/pkg/cmd/api/api.go
- 3: https://man.archlinux.org/man/gh-api.1.en
- 4: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 5: Merge JSON responses from
gh apicli/cli#8620
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant ghJson implementation and the branch-protection read/write paths.
if [ -f templates/branch-protection.js ]; then
echo "== templates/branch-protection.js outline =="
ast-grep outline templates/branch-protection.js --match ghJson --view expanded || true
echo
echo "== relevant ranges =="
sed -n '640,790p' templates/branch-protection.js | nl -ba -v640
fi
echo
echo "== ghJson usages in template =="
rg -n "ghJson\\(|rulesets|allow_force_push|JSON\\.parse" templates/branch-protection.js || true
echo
echo "== config/version references for gh =="
rg -n "github cli|gh |GitHub CLI|cli.github.com|v[0-9]" -S README.md .github package.json 2>/dev/null || trueRepository: DailenG/forge-workflow
Length of output: 314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status and file existence =="
git status --short || true
test -f templates/branch-protection.js && echo "templates/branch-protection.js exists" || echo "templates/branch-protection.js missing"
echo
echo "== awk slice of relevant lines =="
if [ -f templates/branch-protection.js ]; then
awk 'NR>=620 && NR<=805 { printf "%6d\t%s\n", NR, $0 }' templates/branch-protection.js
fi
echo
echo "== search for identifiers in template =="
if [ -f templates/branch-protection.js ]; then
grep -nE "function ghJson|const ghJson|ghJson\\(|rulesets|all=false|allow_force_push|JSON\\.parse|--paginate|--slurp" templates/branch-protection.js || true
fi
echo
echo "== repository docs/package/version references =="
for f in README.md package.json; do
if [ -f "$f" ]; then
echo "== $f =="
grep -nE "github cli|gh |GitHub CLI|cli\\.github\\.com|version|devDependencies|dependencies" "$f" || true
fi
doneRepository: DailenG/forge-workflow
Length of output: 12585
🌐 Web query:
GitHub CLI gh 2.42.0 changelog --paginate --slurp
💡 Result:
The GitHub CLI version 2.42.0 was released on January 11, 2024 [1][2]. The release included the following changes [1]: - Support for template and jq flags when used with the standard format flag. - A fix to prevent the downloading of releases that contain assets matching Windows reserved filenames. - Updated documentation for gh-config to use backquotes for settings [1]. The full changelog comparing version 2.41.0 to 2.42.0 is available on the GitHub repository [1]. Regarding the command line arguments provided in your prompt, please note that the GitHub CLI gh command does not support --paginate or --slurp flags in the way implied for a changelog query [1]. These flags are commonly associated with the gh api command or the jq utility, which is often used in conjunction with gh to process JSON output [1]. If you intended to retrieve the release information programmatically, you would typically use gh release view 2.42.0 or query the GitHub API directly [1].
Citations:
- 1: https://github.com/cli/cli/releases/tag/v2.42.0
- 2: https://mail-index.netbsd.org/pkgsrc-changes/2024/01/12/msg290726.html
🌐 Web query:
site:github.com/cli/cli --slurp --paginate gh api 2.36.0
💡 Result:
The gh api command in the GitHub CLI (gh) uses the --paginate and --slurp flags to handle multi-page API responses [1][2]. --paginate This flag instructs the command to automatically perform additional HTTP requests to fetch all pages of a result set [1][2]. For GraphQL requests, this requires the query to include pageInfo fields (hasNextPage, endCursor) and accept an endCursor variable [1]. --slurp This flag must be used in conjunction with --paginate [1][3]. By default, gh api --paginate outputs pages as individual JSON objects or arrays (JSON Lines style) [1][4]. When --slurp is passed, it wraps all pages of JSON arrays or objects into a single, top-level JSON array [1][2][5]. Important Limitations: Mutual Exclusion: The --slurp flag is currently mutually exclusive with --jq and --template [6][1][3]. If you attempt to use them together, the CLI will return an error stating that the --slurp option is not supported with --jq or --template [6][3][4]. Requirement: The --paginate flag is strictly required when using --slurp [1][3]. If you use --slurp without --paginate, the command will return an error [3].
Citations:
- 1: https://github.com/cli/cli/blob/trunk/pkg/cmd/api/api.go
- 2: Incorrect error shown (the
--slurpoption is not supported with--jqor--template) when passing--jq+--templatewithout--slurpcli/cli#10260 - 3: [gh api] Fix mutual exclusion messages of
--slurpflag cli/cli#10332 - 4: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 5: https://github.com/cli/cli/blob/c14cbaa2/skills/gh/SKILL.md
- 6: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459
🌐 Web query:
"--slurp" "gh api" "2.40" "2.39" "--paginate"
💡 Result:
In the GitHub CLI (gh), the --slurp flag is a specialized option used in conjunction with --paginate to consolidate multiple pages of API responses into a single JSON array [1][2]. Key details regarding its usage and limitations: 1. Purpose: When --paginate is used, the GitHub API typically returns multiple pages as separate JSON objects or arrays (a JSON Lines style output). Passing --slurp wraps these individual pages into a single top-level JSON array, making the output valid, machine-readable JSON [1][3][4]. 2. Limitations: The --slurp flag cannot be used simultaneously with --jq or --template [3][5][4]. The GitHub CLI explicitly enforces this mutual exclusivity in its command processing [4]. 3. Recommended Workaround: If you need to process paginated and slurped data using jq syntax, you should omit --jq from the gh api command and instead pipe the output directly to the standalone jq utility [6][7]. Example command pattern: gh api --paginate --slurp | jq '.' As of the current date (August 2026), these behaviors are standard across recent versions of the GitHub CLI [1][3][4].
Citations:
- 1: https://cli.github.com/manual/gh_api
- 2: https://man.archlinux.org/man/extra/github-cli/gh-api.1.en
- 3: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 4: https://github.com/cli/cli/blob/trunk/pkg/cmd/api/api.go
- 5: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 6: https://github.com/github/gh-cli/blob/v2.94.0/skills/gh/SKILL.md
- 7: rlorenzo/ai-coding-setup@97cacb2
🌐 Web query:
pull 10332 gh api --paginate --slurp merged date
💡 Result:
The --paginate and --slurp flags are options for the GitHub CLI (gh) api command used to fetch and aggregate multiple pages of API results [1][2][3]. --paginate: Automatically makes additional HTTP requests to fetch all pages of results until no more pages remain [1][4]. For GraphQL queries, this requires the query to include pageInfo fields (hasNextPage, endCursor) and accept an $endCursor variable [1][5][6]. --slurp: When used with --paginate, it wraps the individual JSON responses from each page into a single, top-level JSON array, allowing for easier processing with tools like jq [1][3][4]. To fetch the "merged date" of pull requests, you generally query the pull requests endpoint or use GraphQL [7][8][9]. The merged date is available in the merged_at field [10][7][8][9]. Example using gh api (REST) to get merged dates for closed PRs: gh api repos/{owner}/{repo}/pulls --method GET -F state=closed --paginate --slurp | jq '.[] | {number:.number, merged_at:.merged_at}' Example using gh pr list (a higher-level alternative): gh pr list --state merged --json number,mergedAt While --paginate and --slurp are powerful for custom API interactions, consider using gh pr list with the --json flag first, as it provides a simpler interface for common pull request data like mergedAt [7][8][3].
Citations:
- 1: https://cli.github.com/manual/gh_api
- 2: https://www.mankier.com/1/gh-api
- 3: https://github.com/github/gh-cli/blob/v2.94.0/skills/gh/SKILL.md
- 4: https://linuxcommandlibrary.com/man/gh-api
- 5: https://man.archlinux.org/man/gh-api.1.en
- 6: https://man.archlinux.org/man/gh-api.1.en.txt
- 7: https://gist.github.com/m7kvqbe1/81c931719734feefe7ad4bcb8cf903d5
- 8: https://gist.github.com/jason810496/8ed5ec532db5ba666ef8a7becd1eeaae?permalink_comment_id=5680973
- 9: https://stackoverflow.com/questions/79325022/what-would-cause-a-github-prs-merge-commit-sha-to-be-null
- 10: gh search --json state returns "closed" instead of "merged" cli/cli#6290
🌐 Web query:
--paginate JSON Lines gh api output
💡 Result:
When using the --paginate flag with gh api, the GitHub CLI outputs each page of results as a separate, individual JSON object or array, effectively creating a stream of JSON Lines [1][2][3]. To handle this output, you have two primary approaches: 1. Use the --slurp flag If you want to combine all paginated results into a single, valid JSON array, you can use the --slurp flag alongside --paginate [1][4][5]. This instructs gh api to aggregate all pages of JSON arrays or objects into one outer JSON array before printing [1][2]. Note that --slurp is mutually exclusive with --jq and --template [5]. 2. Process with jq If you need to perform filtering or transformation on the output using --jq, you must handle the stream nature of the paginated results [6][7]. Because each page is processed individually by --jq, the output remains in a JSON Lines-like format [6][7]. To aggregate the results into a single array while using jq, you can pipe the output to the standalone jq command with the --slurp (-s) flag [8]: gh api --paginate --jq '' | jq -s '.' In this pipeline, gh api outputs the filtered items page-by-page, and the subsequent jq -s '.' consumes that stream and collects all items into a single final array [8].
Citations:
- 1: https://cli.github.com/manual/gh_api
- 2: https://man.archlinux.org/man/gh-api.1.en.txt
- 3: https://man.archlinux.org/man/gh-api.1.en
- 4: https://cli.github.com/manual/gh_help_reference
- 5: https://github.com/cli/cli/blob/trunk/pkg/cmd/api/api.go
- 6: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 7: Allow
gh api--paginate --slurpto be used with--jqcli/cli#10459 - 8: Output a single JSON document from
api --paginatecli/cli#1268
Combine paginated ruleset pages into one JSON document before parsing.
--paginate prints one JSON object or array per page, but ghJson calls JSON.parse on the whole stdout, so multi-page responses fail and apply may create a duplicate ruleset instead of updating the existing one. Add --slurp here, or avoid --paginate; check the repository’s minimum supported gh version before choosing --slurp.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/branch-protection.js` around lines 679 - 683, Update the ruleset
lookup in the existing `ghJson` call to combine all paginated response pages
into a single JSON document before parsing, using `--slurp` only if supported by
the repository’s minimum `gh` version; otherwise remove pagination or implement
an equivalent aggregation. Preserve the existing lookup behavior so `apply`
reliably detects existing rulesets.
| function readStdin() { | ||
| if (process.stdin.isTTY) { | ||
| return { ok: false, reason: "tty" }; | ||
| } | ||
| try { | ||
| return { ok: true, data: fs.readFileSync(0, "utf8") }; | ||
| } catch (err) { | ||
| return { ok: false, reason: "unreadable", error: err }; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Distinguish an unreadable stdin from an empty stdin in the failure message.
readStdin returns reason: "unreadable" when fs.readFileSync(0) throws, for example on EAGAIN. main then prints MISCONFIGURED_HELP, which states "NO REF UPDATES ON STDIN" and points the user at hook wiring. The wiring is correct in that case, so the message sends the user to the wrong place.
Print the underlying error for the unreadable reason.
🐛 Proposed fix
const stdin = (io && io.readStdin ? io.readStdin : readStdin)();
if (!stdin.ok) {
+ if (stdin.reason === "unreadable") {
+ err(
+ "forge history guard: COULD NOT READ STDIN\n\n " +
+ String(stdin.error && stdin.error.message) +
+ "\n\nRefusing the push rather than allowing an unchecked one.\n"
+ );
+ return EXIT_MISCONFIGURED;
+ }
err(MISCONFIGURED_HELP.join("\n") + "\n");
return EXIT_MISCONFIGURED;
}Also applies to: 273-277
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 216-216: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(0, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/history-guard.js` around lines 212 - 221, Update the main
failure-message handling for the result returned by readStdin so reason
"unreadable" prints the underlying error from its error property instead of
MISCONFIGURED_HELP; preserve the existing message for genuinely empty stdin and
other failure reasons.
| const roots = ["scripts", "templates", "tests", "skills", "hooks", ".github"]; | ||
| const violations = []; | ||
|
|
||
| function walk(rel) { | ||
| const full = path.join(REPO_ROOT, rel); | ||
| for (const entry of fs.readdirSync(full, { withFileTypes: true })) { | ||
| const childRel = path.join(rel, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walk(childRel); | ||
| continue; | ||
| } | ||
| if (!/\.(js|json|md|ya?ml|toml)$/i.test(entry.name)) continue; | ||
| const text = fs.readFileSync(path.join(REPO_ROOT, childRel), "utf8"); | ||
| text.split(/\r?\n/).forEach((line, i) => { | ||
| for (const ch of line) { | ||
| const name = byChar.get(ch); | ||
| if (name) violations.push(childRel + ":" + (i + 1) + ": " + name); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| for (const root of roots) walk(root); | ||
| for (const file of ["README.md", "CHANGELOG.md", "CONTRIBUTING.md", "CLAUDE.md"]) { | ||
| const text = read(file); | ||
| text.split(/\r?\n/).forEach((line, i) => { | ||
| for (const ch of line) { | ||
| const name = byChar.get(ch); | ||
| if (name) violations.push(file + ":" + (i + 1) + ": " + name); | ||
| } | ||
| }); | ||
| } | ||
| assert.deepEqual(violations, []); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover all claimed working-tree files.
This test claims to inspect committed and uncommitted files. It omits .claude-plugin, docs, and the root package.json. Its extension filter also omits .html. An untracked file in those locations can contain forbidden typography and evade this test and typography-check.js, which only uses git ls-files.
Proposed fix
- const roots = ["scripts", "templates", "tests", "skills", "hooks", ".github"];
+ const roots = [
+ "scripts", "templates", "tests", "skills", "hooks",
+ ".github", ".claude-plugin", "docs",
+ ];
@@
- if (!/\.(js|json|md|ya?ml|toml)$/i.test(entry.name)) continue;
+ if (!/\.(js|json|md|html|ya?ml|toml)$/i.test(entry.name)) continue;
@@
- for (const file of ["README.md", "CHANGELOG.md", "CONTRIBUTING.md", "CLAUDE.md"]) {
+ for (const file of [
+ "README.md", "CHANGELOG.md", "CONTRIBUTING.md", "CLAUDE.md", "package.json",
+ ]) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const roots = ["scripts", "templates", "tests", "skills", "hooks", ".github"]; | |
| const violations = []; | |
| function walk(rel) { | |
| const full = path.join(REPO_ROOT, rel); | |
| for (const entry of fs.readdirSync(full, { withFileTypes: true })) { | |
| const childRel = path.join(rel, entry.name); | |
| if (entry.isDirectory()) { | |
| walk(childRel); | |
| continue; | |
| } | |
| if (!/\.(js|json|md|ya?ml|toml)$/i.test(entry.name)) continue; | |
| const text = fs.readFileSync(path.join(REPO_ROOT, childRel), "utf8"); | |
| text.split(/\r?\n/).forEach((line, i) => { | |
| for (const ch of line) { | |
| const name = byChar.get(ch); | |
| if (name) violations.push(childRel + ":" + (i + 1) + ": " + name); | |
| } | |
| }); | |
| } | |
| } | |
| for (const root of roots) walk(root); | |
| for (const file of ["README.md", "CHANGELOG.md", "CONTRIBUTING.md", "CLAUDE.md"]) { | |
| const text = read(file); | |
| text.split(/\r?\n/).forEach((line, i) => { | |
| for (const ch of line) { | |
| const name = byChar.get(ch); | |
| if (name) violations.push(file + ":" + (i + 1) + ": " + name); | |
| } | |
| }); | |
| } | |
| assert.deepEqual(violations, []); | |
| const roots = [ | |
| "scripts", "templates", "tests", "skills", "hooks", | |
| ".github", ".claude-plugin", "docs", | |
| ]; | |
| const violations = []; | |
| function walk(rel) { | |
| const full = path.join(REPO_ROOT, rel); | |
| for (const entry of fs.readdirSync(full, { withFileTypes: true })) { | |
| const childRel = path.join(rel, entry.name); | |
| if (entry.isDirectory()) { | |
| walk(childRel); | |
| continue; | |
| } | |
| if (!/\.(js|json|md|html|ya?ml|toml)$/i.test(entry.name)) continue; | |
| const text = fs.readFileSync(path.join(REPO_ROOT, childRel), "utf8"); | |
| text.split(/\r?\n/).forEach((line, i) => { | |
| for (const ch of line) { | |
| const name = byChar.get(ch); | |
| if (name) violations.push(childRel + ":" + (i + 1) + ": " + name); | |
| } | |
| }); | |
| } | |
| } | |
| for (const root of roots) walk(root); | |
| for (const file of [ | |
| "README.md", "CHANGELOG.md", "CONTRIBUTING.md", "CLAUDE.md", "package.json", | |
| ]) { | |
| const text = read(file); | |
| text.split(/\r?\n/).forEach((line, i) => { | |
| for (const ch of line) { | |
| const name = byChar.get(ch); | |
| if (name) violations.push(file + ":" + (i + 1) + ": " + name); | |
| } | |
| }); | |
| } | |
| assert.deepEqual(violations, []); |
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 83-83: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(rel, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(zip-slip-archive-extraction-javascript)
[warning] 89-89: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(path.join(REPO_ROOT, childRel), "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/repo-standards.test.js` around lines 78 - 110, Update the repository
traversal test around roots, explicit root files, and the extension filter so it
covers .claude-plugin and docs directories, the root package.json, and HTML
files. Ensure both tracked and untracked working-tree files in these locations
are scanned for forbidden characters, while preserving the existing violation
reporting and assertion behavior.
1 participant
What this changes
Phase 2 required a GitHub ruleset on
main. GitHub reserves that for paid plans on private personal repositories and answersUpgrade to GitHub Pro or make this repository public to enable this feature, so a solo developer on a free plan could only pass the bootstrap gate by paying or by publishing a private repository. Neither is an acceptable price for a lifecycle gate.The requirement is now stated behaviourally, with no host in it: the default branch must not be deletable and must not accept a non-fast-forward update, while ordinary fast-forward pushes and this workflow's
--no-ffmerges keep working. It is satisfied at the strongest tier the host and account actually grant.Tier 1, server side. GitHub rulesets, falling back to classic branch protection on older Enterprise Server, and GitLab protected branches. Self-hosted and unrecognised hosts skip straight to tier 2.
Tier 2, managed local.
.forge/history-guard.jsas apre-pushhook, reading git's ref-update records from stdin. Deletion of the protected branch and non-fast-forward updates are refused; fast-forward pushes and initial branch creation pass; empty or malformed input fails closed with a message naminguse_stdin: trueas the fix.The gate item
default-branch history protection verifiedaccepts either tier, tier 2 only with its narrower trust boundary recorded. An unavailable paid hosting feature is no longer a fatal bootstrap failure.Repository visibility
Never changed. Every provider call goes through one choke point that refuses to issue a visibility mutation at all, including the
ghforms that are easy to miss (-X PATCH, and a body-bearing call with no explicit method, whichghsilently upgrades to PATCH).Verification
Runs against disposable repositories under the system temp directory, never a real remote. The bare stand-in remote has its
HEADparked offmain, so git's own "refusing to delete the current branch" cannot mask a guard that does nothing. Every recursive delete is refused unless the path resolves to a directory the tool itself created with its own prefix, and is not the temp root, home, or the working directory.Migration
Projects blocked on a paid-plan ruleset resume with
node .forge/branch-protection.js migrate, which re-detects capability, stands up and verifies the fallback, updates the state file it owns, and returns a plan naming exactly which recorded blocker to clear. Unrelated blockers are preserved. Lifecycle files are left for forge to edit.Testing
node --test: 94 tests, 0 failures. Suites cover remote selection, plan and permission rejection, unknown-provider fallback, visibility preservation, all four push decisions, stdin propagation through the hook manager, a starved guard failing closed, disposable-remote end to end, gate acceptance at either tier, state file and environment report output, migration from a paid-ruleset block, and no regression to the existing manifest, typography, and syntax checks.A review pass after the suite was first green found three ways verification could pass vacuously, all fixed and now covered by tests:
verifycalled the installer, so a hook the user deleted was silently recreated and then reported verified. It now inspects only.core.hooksPathand trustedlefthook.ymlwithout checkinglefthook installhad run.priority, then the leading number in the command name, then alphabetically, so a guard named99_historylisted first was accepted while running last.piped: truewas also missing, without which the build and tests still run after a refusal.Known difference between the tiers
Tier 2 protects configured clones. It does not stop a push from an unconfigured clone, a write through the host's web UI or API, a hook that was deleted or edited, or an attacker holding valid credentials. Tier 1 has none of those gaps. That limitation is recorded in the state file, the environment report, the decision record, and stated once to the user.
Checklist
claude plugin validate --strict .passes.claude-plugin/plugin.json(1.0.0 to 1.1.0)CHANGELOG.mdupdated under a version heading🤖 Generated with Claude Code