feat: bootstrap phase 6 reports and offers repository security settings - #27
Conversation
…efaults Each default contradicted something the repository already documents. squash_merge_commit_title=PR_TITLE. AGENTS.md, CONTRIBUTING.md, pr-authoring and branch-and-commit all state the PR title becomes the commit message on main. GitHub's COMMIT_OR_PR_TITLE default makes that false whenever a PR has exactly one commit -- visible in this repo's own log, where #7 and #9 carry the (#N) suffix and #11, #13 and #15 do not. squash_merge_commit_message=PR_BODY. The COMMIT_MESSAGES default concatenates every branch commit message into the main commit body, and release-please deliberately parses that body for further Conventional Commits and BREAKING-CHANGE footers. Already live here: f98cbf9's body carries "* chore: trigger CI on release PR", harmless only because chore is release-please's hidden bucket. A branch commit reading "fix: wip" would have produced a phantom changelog entry or an unintended bump. allow_rebase_merge=false. The comment justifying rebase claimed release-please merges its own PR. It does not -- ADR-0002, the release-please workflow header and the release-management skill all require a human merge. Nor has rebase ever been used: the history has no merge commits, all eight merged PRs were squash-merged, and both release PRs cut their tags that way. allow_update_branch=true. The ruleset sets strict_required_status_checks_policy=false, so the "Update branch" button is not offered at all without it. Also corrects skills/release-management/SKILL.md, which instructed `gh pr merge --merge`. allow_merge_commit=false has been set since day one, so that command has always returned HTTP 405 -- the documented release procedure was broken. Its parenthetical was wrong too: release-please does not merge. Accepted cost: with PR_BODY an unmodified PR template lands verbatim in the main commit message. BLANK was rejected -- it drops Co-authored-by trailers and discards the RISK/rollback record. PR_BODY reduces the misparse surface but does not eliminate it; a Conventional-Commit-shaped line in a PR body is still parsed. Closes #24 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bootstrap did nothing about security settings -- grep for secret, scanning,
visibility, vulnerability or dependabot in the script returned nothing. Two
consequences. Converting a repo private to public grants ACCESS to secret
scanning but does not enable it, and push protection in particular must be
switched on explicitly, which is how a public repo ends up without it. And
.github/dependabot.yml asserts Dependabot alerts and security updates are
enabled while nothing verifies that.
The write path is bounded by cost, not by capability: the only settings this
phase ever enables are free by construction.
- Public repo: offers secret scanning and push protection, both free.
- Private or internal: never writes them, under any flag. There they need a
paid Advanced Security / Secret Protection seat, and a setup script must not
commit an adopter's account to a per-committer charge. It reports the state
and emits a MANUAL step.
- Dependabot alerts and automated security fixes: free everywhere, so offered
regardless of visibility.
- Repository visibility is never changed and never offered. Private to public
erases stars and watchers and publishes all Actions history -- a one-way
door, and the phase comment says so, because "detect whether the repo is
public" invites someone to add that prompt later.
Two states the GitHub UI blurs are kept distinct. Unreadable settings mean the
token lacks admin, not that the setting is off, so security_and_analysis
coming back null degrades to warn plus MANUAL rather than reporting "disabled".
And automated-security-fixes returns {"enabled":..,"paused":..}: enabled but
paused means no fix PR ever opens, so it is reported separately.
Verified read-only before writing the phase: gh api documents the
key[subkey]=value nested syntax, so the security_and_analysis PATCH stays a
normal run_or_dry call visible under --dry-run; /vulnerability-alerts returns
204 enabled and 404 disabled; /automated-security-fixes returns enabled and
paused.
Unlike phases 3/5/8 this phase runs several independent checks, so it
accumulates a result and calls record_phase once at the end -- the pattern
phase_issue_types uses, matching run_phase's one-record-per-exit-path
contract.
Phases 6/7/8 renumber to 7/8/9 across the script and docs. Because every read
runs for real under --dry-run, bootstrap --dry-run now doubles as a zero-risk
security audit.
Closes #26
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a43bc8fb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$visibility" = "public" ]; then | ||
| if [ "$secret_scanning" = "enabled" ] && [ "$push_protection" = "enabled" ]; then | ||
| ok "secret scanning + push protection: already enabled" | ||
| else |
There was a problem hiding this comment.
When a caller can read the public repository but lacks permission to view security_and_analysis, the query produces public\tunknown\tunknown, so facts is nonempty and this branch treats both unknown values as disabled. The script then prompts to enable the settings—or attempts the PATCH automatically under --yes—instead of reporting the unreadable state and issuing the documented manual step. Add an explicit unknown path before deciding whether either setting is off.
Useful? React with 👍 / 👎.
| updates". On a private repo the first two require a Secret Protection licence; | ||
| the Dependabot pair are free everywhere. | ||
|
|
||
| ### 7. Actions PR permission |
There was a problem hiding this comment.
After renumbering the Actions permission step to phase 7, the troubleshooting section at lines 268–271 still says the failure means phase 6 was skipped or declined and tells users to accept the phase 6 prompt. Phase 6 is now Security, so this recovery guidance directs users to the wrong prompt; update both references to phase 7.
Useful? React with 👍 / 👎.
# Conflicts: # docs/setup/bootstrap.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 978df54611
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if run_or_dry gh api -X PATCH "repos/${REPO}" \ | ||
| -f 'security_and_analysis[secret_scanning][status]=enabled' \ | ||
| -f 'security_and_analysis[secret_scanning_push_protection][status]=enabled'; then | ||
| ok "secret scanning + push protection enabled" |
There was a problem hiding this comment.
When any security setting is disabled and the script runs with --dry-run (especially with --yes), run_or_dry performs no mutation but returns success, so this line reports the protections as enabled and the phase summary remains green. The same fall-through occurs for Dependabot alerts at line 807 and security updates at line 836. This makes the documented security-audit mode misleading: its final status contradicts the real reads and can leave an operator believing disabled protections are active.
Useful? React with 👍 / 👎.
1. An unreadable secret-scanning state was treated as disabled. security_and _analysis is only populated for callers with admin on the repo, so a token without it yields "public\tunknown\tunknown" -- a non-empty facts line that fell through to the else branch and prompted to enable, or PATCHed outright under --yes. That acts on a guess and reports a state never observed, which is the opposite of the unknown-is-not-disabled rule this phase was written around. There is now an explicit unknown branch that warns, emits a manual step, and writes nothing. 2. The Actions-permission troubleshooting entry still said phase 6 after the renumber moved it to 7, so the recovery text pointed at the Security prompt instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 656e4ef627
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else | ||
| warn "repository is ${visibility}: secret scanning (${secret_scanning}), push protection (${push_protection})" | ||
| warn " on a private/internal repo these need a paid GitHub Advanced Security / Secret Protection seat" | ||
| warn " bootstrap will not enable them for you — that is a billing decision, not a setup step" | ||
| manual "Private repo: decide whether to license GitHub Secret Protection, then enable secret scanning + push protection in Settings → Advanced Security" |
There was a problem hiding this comment.
When a private or internal repository already has licensed Secret Protection and both statuses read enabled, this unconditional branch still marks phase 6 as a warning and queues an instruction to license and enable them. A fully configured repository therefore cannot receive a clean idempotent audit and the operator is told to perform an already-completed action; handle the both-enabled state before emitting the visibility warning.
Useful? React with 👍 / 👎.
Codex P2s addressed1. Unreadable state treated as disabled. Correct, and it broke the rule this phase was written around. Verified with a
2. Stale phase-6 reference. Fixed — the Actions-permission troubleshooting entry now says phase 7 in both places. |
# Conflicts: # docs/setup/bootstrap.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f97592e96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # wording covers "or not visible" rather than asserting the wrong one. | ||
| if gh api "repos/${REPO}/vulnerability-alerts" >/dev/null 2>&1; then | ||
| ok "Dependabot alerts: enabled" | ||
| elif confirm "Enable Dependabot alerts? (free on every plan; .github/dependabot.yml assumes it)" "y"; then |
There was a problem hiding this comment.
When the token can read the repository but lacks admin access, this GET can fail with 403 exactly as the preceding comment notes, yet the failure immediately triggers an enable prompt and, under --yes, an unauthorized PUT. Declining also emits a manual enable step even though the setting may already be enabled. The automated-security-fixes check at lines 832–845 has the same problem, so both failed reads should be reported as unknown rather than treated as disabled.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds bootstrap phase 6: reports repository visibility and four security settings, and offers to enable the ones that are free. Renumbers phases 6/7/8 to 7/8/9.
Related issue
Closes #26
Validation
make test(placeholder; nothing coversbootstrap.sh, see RISK)bash -nclean,shellcheck -s bashclean, API shapes verified read-only,--dry-runrun against this repoEvery API shape was verified read-only before the phase was written, not assumed:
gh api --helpkey[subkey]=valuenested syntax is documentedGET .../vulnerability-alertsHTTP/2.0 204 No Content, exit 0 when enabledGET .../automated-security-fixes{"enabled":true,"paused":false}GET repos/{o}/{r} --jq '[...]'public enabled enabledThe nested-syntax one is load-bearing: it means the
security_and_analysisPATCH stays a normalrun_or_dry gh apicall, visible under--dry-run, instead of needing a piped JSON body that would bypass the dry-run choke point.Dry run against this repo (public, all four already on — so the correct behaviour is four
oklines, zero prompts, zero writes):And the summary confirms all ten phases record exactly once with the new numbering:
Risk / rollback
Checklist
*.local.mdfiles committeddocs/setup/bootstrap.md(new### 6. Security, renumbering, troubleshooting entry),SECURITY.md(new section),docs/template/architecture.md(release checklist)The design rule, which is the part to review
The only settings this phase ever enables are free by construction. Anything with a billing consequence is reported and handed back as a MANUAL step.
--yesenables — no more aggressive than phase 7, which already grants Actions the power to create and approve PRs under--yes..github/dependabot.yml:9-10has always asserted both are on; this is the first thing that checks.Two states the GitHub UI blurs, kept distinct
Unreadable ≠ disabled.
security_and_analysisis only populated for callers with admin permission — it comes back null otherwise. Reporting that as "disabled" would be a lie that reads as reassuring in the wrong direction, so it degrades towarn+manualwith an explicit "the token may lack admin" message, and a matching troubleshooting entry.Enabled ≠ unpaused.
automated-security-fixesreturnspausedas well asenabled. Enabled-but-paused means no fix PR will ever open, so it gets its ownwarnrather than being folded into "enabled".Structural notes
gh apiprints HTTP error bodies to stdout, so|| truewould capture the error body as data.$resultand callsrecord_phaseonce at the end (the patternphase_issue_typesuses).run_phase's contract wants one record per exit path, not one per check.--skip-securityflag: the phase never writes without a prompt and never writes anything with a cost, so there is nothing to skip.One documented false alarm
GitHub's "Setting repository visibility" doc lists "all push rulesets will be disabled" among the consequences of going public. An adopter reading that will reasonably fear for their
mainprotection..github/rulesets/main-branch.jsonis"target": "branch", not"push", so it is unaffected. One sentence in the phase-6 docs says so — the rare case where restating an external fact earns its keep, because the value is in the negation.