Skip to content

chore(ci): keep the release token out of the push URL; website install without scripts - #889

Merged
V3RON merged 1 commit into
mainfrom
security/ci-hygiene
Oct 26, 2026
Merged

V3RON merged 1 commit into
mainfrom
security/ci-hygiene

Conversation

@V3RON

@V3RON V3RON commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two CI hardening fixes. The Release workflow no longer puts GITHUB_TOKEN in the push URL. The website job installs with --ignore-scripts, like every other job.

Changes

  • .github/workflows/release.yml (Push step): the remote is plain https://github.com/${GITHUB_REPOSITORY}.git. The token goes as the AUTHORIZATION: basic … extraheader that actions/checkout v7.0.1 uses, set through GIT_CONFIG_COUNT / GIT_CONFIG_KEY_0 / GIT_CONFIG_VALUE_0, so it is in neither the URL nor git's argv. The base64 value goes to ::add-mask::. --atomic and the lease stay.
  • .github/workflows/website.yml: pnpm install --frozen-lockfile --ignore-scripts.
  • src/source-hygiene.test.ts: every workflow pnpm install passes --ignore-scripts; no URL holds a credential; the Release push keeps the token in GIT_CONFIG_*, plus --atomic, the lease and the refspecs.

Verified

  • A local HTTP server received the expected header from git push with only the GIT_CONFIG_* variables set.
  • The website builds from a fresh install without scripts (72 pages, check-openapi ok).
  • Playwright installs are unchanged: the locked playwright 1.63.0 already fixes the Chromium build.

Assumptions

  • Assumption: pass the credential through git -c or through GIT_CONFIG_* environment variables? — GIT_CONFIG_*, because -c still puts the base64 token in git's argv.
  • Assumption: change the Playwright installs? — no; this PR only records why they are fine.

Review

Spec review: skipped. Code review: 0 blocking, 0 fixed, 0 notes. Claims review: skipped.
Rounds: 1. Tokens: n/a.

One combined review by the maintainer's agent on 2026-10-22 (comment below); no blocking findings.

Rebased onto main e1f2f43 on 2026-10-26 as one commit. Conflict in src/source-hygiene.test.ts: main's vendored-inventory tests and this PR's workflow tests kept side by side.

Written by an agent.

@V3RON

V3RON commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Review (maintainer's agent, round 1)

No blocking findings.

  • The GIT_CONFIG_* form is right and better than the git -c form the brief suggested: the credential is in neither the URL nor argv, and ::add-mask:: covers the base64 value that Actions would otherwise print. Key and value match actions/checkout v7.0.1's git-auth-helper.ts.
  • --ignore-scripts on the website install is proven by a fresh build (72 pages, check-openapi ok); pnpm 11 already skipped dependency builds there without allowBuilds, so this is explicitness, not a behaviour change.
  • Tests pin the shape that matters (no credential in any workflow URL, every pnpm install without scripts, the push step's header plumbing and its --atomic lease) and fail on main.

Notes, no change needed: the Playwright browser revision follows the locked playwright 1.63.0 as stated; apt packages from --with-deps stay unpinned by nature.

@V3RON
V3RON force-pushed the security/ci-hygiene branch from b168e30 to da64a99 Compare October 26, 2026 14:25
@V3RON
V3RON merged commit ce9413f into main Oct 26, 2026
25 checks passed
@V3RON
V3RON deleted the security/ci-hygiene branch October 26, 2026 14:26
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.

1 participant