Skip to content

feat(release): refuse to publish until GitHub enforces the release protections - #3388

Merged
thymikee merged 2 commits into
feat/release-workflowfrom
feat/release-protection-check
Oct 11, 2026
Merged

thymikee merged 2 commits into
feat/release-workflowfrom
feat/release-protection-check

Conversation

@thymikee

@thymikee thymikee commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Summary

npm trusts every release.yml run that reaches the npm-publish environment, and the plan a run executes is that run's own copy, so it can't protect anything by itself. Before a nightly or stable publish, the plan now reads the repository's settings through the public API. It refuses, and names each gap, unless all of these hold. Only a 404 counts as "not configured"; any other API error fails the plan.

  • npm-publish deploys only from the branch main and the tag pattern v*
  • the release environment requires a reviewer
  • an active tag ruleset restricts creating, updating, and deleting every refs/tags/v* tag, with no exclusions and, where GitHub shows its bypass list, only admin bypass

This makes merging the stack before the one-time setup safe: the release workflow fails until the setup is done. #3381 documents the setup. 2 files.

Validation

At ae04bddfd3:

  • release-plan.test.ts passes 29/29 (with the stacked tests). Each protection gap is named on its own.
  • Fallow, lint, and format pass.
  • Run read-only against today's repo, the check refuses with all three gaps.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.16 MB 5.16 MB 0 B
Package (unpacked) 5.16 MB 5.16 MB 0 B
Package (download) 1.55 MB 1.55 MB +6 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.9 ms 25.5 ms -0.4 ms
CLI --help 77.1 ms 74.9 ms -2.2 ms

@thymikee
thymikee force-pushed the feat/release-protection-check branch from 506da75 to 8e2a549 Compare October 10, 2026 17:42
@thymikee
thymikee marked this pull request as ready for review October 10, 2026 19:08

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread scripts/release-plan.mjs Outdated
Comment thread scripts/release-plan.mjs
Comment thread scripts/__tests__/release-plan.test.ts
Comment thread scripts/release-plan.mjs Outdated
Comment thread scripts/release-plan.mjs
@thymikee

Copy link
Copy Markdown
Member Author

The release gate in 8e2a549 does what the PR describes, but one Cubic thread still holds: guardsReleaseTags can accept a tag ruleset that does not cover every v* tag. Left as is, a ruleset with an exclude pattern or a broad bypass list would pass the check, and the gate would not protect tags the way the release depends on. The fix is one rule: a ruleset counts only if every ref matching refs/tags/v* is covered, so no exclude pattern overlaps it and bypass_actors is empty or admin-level only (return a gap when bypass_actors is missing from the response). Please also catch only 404s in readRepositorySetup, and add one test per weakening field.

Open review threads: the exclude handling and the all-gaps test still apply, as does one lower-priority thread. The thread on token permissions for the rulesets read does not apply, because the rulesets list needs only metadata read, and the Plan job already grants actions:read for the environment reads. The thread on the per-id ruleset fetch does not apply either, because the list endpoint returns summaries without rules or conditions, so that second fetch is required. Please resolve both.

Not blocking, and fine to take or leave: the refusal message points to a CONTRIBUTING.md "One-time repository setup" section that lands in #3381, so merge that first or together with this PR; and no test reaches readRepositorySetup or the mode gate in main(), so narrowing the check to nightly would keep the suite green, which an injectable fetch plus one stable-mode refusal test would catch.

The eight non-passing checks were all cancelled by newer runs, with no failed step. Lint & Format, Typecheck and Coverage run the changed test file, so please let a rerun finish. I did not run the tests or call the live GitHub API. I could not confirm that the Plan job's token sees every ruleset field, but a hidden field blocks the release and does not let it through. The first nightly after the one-time setup will show the pass path. There are no conflicts.

@thymikee
thymikee force-pushed the feat/release-protection-check branch 2 times, most recently from 212c05f to 63dca49 Compare October 11, 2026 06:55
…otections

npm trusts every release.yml run that reaches the npm-publish environment, and
a run's own copy of the plan proves nothing. Before a nightly or stable
publish, the plan reads the repository's settings and refuses unless
npm-publish deploys only from main and v* tags, the release environment
requires a reviewer, and an active tag ruleset restricts creating, updating,
and deleting v* tags.
…on API errors

A tag ruleset counts only with no exclusions and, where GitHub shows its bypass
list, only admin bypass. A missing environment is a setup gap only on a 404;
any other API failure now fails the plan instead of reading as a gap.
@thymikee
thymikee force-pushed the feat/release-protection-check branch from 63dca49 to ae04bdd Compare October 11, 2026 07:01
@thymikee

Copy link
Copy Markdown
Member Author

Fixed in ae04bddfd3:

  • A tag ruleset now counts only with no exclusions, and, where GitHub shows the bypass list, only admin bypass.
  • Only a 404 counts as a missing environment or policy list; any other error fails the plan.
  • There is a test for each weakening field: custom_branch_policies: false with exact policies, an exclusion, and a writer bypass. An admin bypass passes.

I did not treat a missing bypass_actors as a gap. GitHub includes that field only for admins (checked: unauthenticated and non-admin reads omit it), so that rule could never pass in CI and would block every release.

The per-ruleset fetch and token-permission threads are answered and resolved as not applicable.

Not taken: an injectable fetch to test readRepositorySetup and the mode gate in main(). A change that narrowed the gate to nightly would still pass the suite, but the gate is one line and the live read-only run against the repo refuses with all three gaps. #3381 merges together with this PR in the stack, so the CONTRIBUTING link resolves.

@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready at ae04bdd. The earlier findings from 8e2a549 are fixed, and I found no new problem in the changes since then. Not blocking, and you can take or leave these: the Plan job runs with the read-only GITHUB_TOKEN, so bypass_actors is likely hidden there and the admin-only bypass check at scripts/release-plan.mjs:131 may pass without real evidence, so the gap text or the CONTRIBUTING setup section could say the bypass list is checked only when it is visible (I could not confirm what GitHub returns for that token without a live API read); and scripts/tests/release-plan.test.ts:155 has no case for an include list without refs/tags/v* (for example refs/tags/release-*) and none for the ~ALL branch, so one case each would close that.

On the other threads, the cubic-dev-ai threads on the ref_name.exclude check and the custom_branch_policies test are fixed at this head, and the one on 404-only handling is fixed too, so you can resolve all three. The thread on unauthenticated ruleset reads does not apply, because a 403 would throw uncaught and fail closed without publishing. The thread on the per-id ruleset fetch does not apply either, because the list endpoint returns summaries without rules, so the extra read-only calls are needed. You can resolve both.

CI is likely unrelated to this change. Smoke Tests and Bundle Size never load scripts/release-plan.mjs, and both were cancelled rather than failed, which looks like a superseded run. Before merge, the cancelled Smoke Tests and Bundle Size jobs need a green rerun, and this should merge together with #3381 so the CONTRIBUTING link resolves. No conflicts are known.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 11, 2026
@thymikee
thymikee merged commit feec689 into main Oct 11, 2026
32 of 34 checks passed
@thymikee
thymikee deleted the feat/release-protection-check branch October 11, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant