diff --git a/.github/workflows/publish-crates.yml b/.github/workflows/publish-crates.yml index 3743614..514e40c 100644 --- a/.github/workflows/publish-crates.yml +++ b/.github/workflows/publish-crates.yml @@ -4,9 +4,9 @@ name: Publish to crates.io # version tag but does not publish to crates.io. This companion workflow does # that, on the same tag, so a release still lands on crates.io as before. # -# Release flow (maintainer): bump `version` in Cargo.toml, then push the -# matching `vX.Y.Z` tag. That tag fires both `release.yml` (binaries + -# installers) and this workflow (crates.io). +# Release flow (maintainer): merge a version-bump PR labelled `release` into +# main. `release-tag.yml` tags `vX.Y.Z` from that merge, and the tag fires both +# `release.yml` (binaries + installers) and this workflow (crates.io). # # Security: publishing is a maintainer-only action. It is gated by (1) tag # protection — only admins can create `v*` tags (repo ruleset); (2) the diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml new file mode 100644 index 0000000..9369be7 --- /dev/null +++ b/.github/workflows/release-tag.yml @@ -0,0 +1,131 @@ +name: release-tag + +# The release PR *is* the release. +# +# Merging a version-bump PR into `main` tags `vX.Y.Z` from here, and that tag is +# what drives the rest of the pipeline (`release.yml` builds the binaries and +# installers, `publish-crates.yml` publishes to crates.io). This keeps tagging +# inside the maintainer-gated PR flow instead of a separate, manual +# `git tag && git push` step that has to be remembered — and kept correct — +# after every release merge. +# +# Security notes, because this workflow holds the keys to a release: +# +# * It triggers on `push`, never on `pull_request`. For a `push` event GitHub +# always takes the workflow definition from the pushed branch (`main`), so a +# pull request branch cannot edit this file to make it do something else +# with the token. A `pull_request`-triggered workflow runs the *PR's* copy of +# the definition, which would be exactly the wrong property here. +# * The push must be the repository owner's (`github.actor` guard), the commit +# must belong to a merged PR carrying the `release` label (the same label +# `version-guard.yml` requires before it will allow a version bump at all), +# and the version must actually have changed. Anything else is a no-op. +# * Tag creation only ever adds a new tag; an existing tag is never moved. +# * The tag is pushed with a maintainer PAT (`secrets.RELEASE_TAG_TOKEN`), +# not `GITHUB_TOKEN`, for two reasons: a `GITHUB_TOKEN` push does not +# trigger further workflow runs (so `release.yml` would never fire), and the +# `release-tags` ruleset restricts tag creation to admins. Using the owner's +# PAT also keeps `github.actor` on the tag push equal to the owner, so +# `publish-crates.yml`'s owner guard still holds. +# +# This workflow never publishes anything itself — it only creates the tag. +# See CONTRIBUTING.md ("Release security") for the setup and the full model. + +on: + push: + branches: [ "main" ] + paths: + - "Cargo.toml" + +# The tag push uses the PAT below, so GITHUB_TOKEN stays read-only here. +permissions: + contents: read + pull-requests: read + +jobs: + tag: + name: Tag the merged release PR + # Defense-in-depth: only the owner's merge can cut a release. + if: github.actor == github.repository_owner + runs-on: ubuntu-latest + # Scopes RELEASE_TAG_TOKEN to this workflow (and lets the owner add a + # required reviewer if they want a second OK before a release goes out). + environment: release-tag + + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Decide whether this push cuts a release + id: decide + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BEFORE: ${{ github.event.before }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + + extract() { grep -E -m1 '^version[[:space:]]*=' | sed -E 's/.*"([^"]+)".*/\1/'; } + + VERSION="$(extract < Cargo.toml)" + TAG="v$VERSION" + { + echo "version=$VERSION" + echo "tag=$TAG" + } >> "$GITHUB_OUTPUT" + + skip() { echo "cut=false" >> "$GITHUB_OUTPUT"; echo "::notice::$1"; exit 0; } + + # `paths: Cargo.toml` also fires for dependency edits, so compare the + # version against the previous state of main. `github.event.before` is + # unusable after a force-push or on a branch's first push; fall back to + # this commit's first parent, which for a PR merge is main-before. + PREV_REF="$BEFORE" + if [ -z "$PREV_REF" ] || ! git cat-file -e "${PREV_REF}^{commit}" 2>/dev/null; then + PREV_REF="${GITHUB_SHA}^" + fi + PREV_VERSION="$(git show "$PREV_REF:Cargo.toml" | extract)" + echo "previous=$PREV_VERSION current=$VERSION" + if [ "$PREV_VERSION" = "$VERSION" ]; then + skip "Cargo.toml changed but the version is still $VERSION — nothing to tag." + fi + + # Tie the tag to a release PR: the `release` label is what allowed the + # bump past version-guard in the first place, and applying it needs + # write access. + if ! gh api "repos/$REPO/commits/$GITHUB_SHA/pulls" --jq '.[].labels[].name' \ + | grep -qx 'release'; then + skip "No merged PR labelled 'release' for $GITHUB_SHA — refusing to tag $TAG." + fi + + # Idempotent: only ever add a tag, never move one. + if git ls-remote --exit-code --tags origin "refs/tags/$TAG" >/dev/null 2>&1; then + skip "Tag $TAG already exists — nothing to do." + fi + + echo "cut=true" >> "$GITHUB_OUTPUT" + + - name: Push the release tag + if: steps.decide.outputs.cut == 'true' + env: + TAG: ${{ steps.decide.outputs.tag }} + VERSION: ${{ steps.decide.outputs.version }} + TOKEN: ${{ secrets.RELEASE_TAG_TOKEN }} + run: | + set -euo pipefail + + if [ -z "${TOKEN:-}" ]; then + echo "::error::RELEASE_TAG_TOKEN is not configured, so $TAG cannot be pushed in a way that triggers release.yml. See CONTRIBUTING.md ('Release security')." + exit 1 + fi + + git config user.name 'github-actions[bot]' + git config user.email 'github-actions[bot]@users.noreply.github.com' + git tag -a "$TAG" -m "Release $VERSION" + # Actions masks the secret in logs; the URL form keeps the PAT out of + # .git/config (checkout ran with persist-credentials: false). + git push "https://x-access-token:${TOKEN}@github.com/${GITHUB_REPOSITORY}.git" "refs/tags/$TAG" + + echo "::notice::Pushed $TAG — release.yml and publish-crates.yml take it from here." diff --git a/.github/workflows/version-guard.yml b/.github/workflows/version-guard.yml index b433145..a8440d7 100644 --- a/.github/workflows/version-guard.yml +++ b/.github/workflows/version-guard.yml @@ -1,13 +1,15 @@ name: version-guard -# Releasing is a maintainer-only action: the maintainer bumps the crate version -# in Cargo.toml and pushes a matching `vX.Y.Z` tag, which triggers the release -# pipeline (dist-generated release.yml builds binaries + installers, and -# publish-crates.yml publishes to crates.io). To keep version bumps controlled, -# this check fails any pull request that changes the `version` field. +# Releasing is a maintainer-only action: the maintainer merges a version-bump PR +# into main, and `release-tag.yml` turns that merge into the matching `vX.Y.Z` +# tag, which triggers the release pipeline (dist-generated release.yml builds +# binaries + installers, and publish-crates.yml publishes to crates.io). Since a +# merged bump now cuts a release on its own, this check fails any pull request +# that changes the `version` field. # # Maintainer escape hatch: add the `release` label to a PR to allow the version -# bump (e.g. the release PR itself). +# bump (e.g. the release PR itself). That same label is what `release-tag.yml` +# requires before it will tag, so this label is the single opt-in for a release. on: pull_request: diff --git a/CLAUDE.md b/CLAUDE.md index 8448142..ff030da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,8 +35,9 @@ cargo run --bin scope -- completions zsh # emit a shell completion script - Optional config file (`infra/config.rs`): `/scope/config.toml` (e.g. `~/.config/scope/config.toml`, alongside the crash backups). Supports `capacity`, `tag_file`, and an optional `[shortcuts]` table (see below). Resolution precedence is **CLI flag > config.toml > built-in default** (`Config::load` is folded into `main`'s single fatal-error flow). A missing file/field falls through to defaults; a malformed file or unknown key is a fatal error (`deny_unknown_fields`). Path values (`tag_file`) are used verbatim — there is no shell involved, so `~` and `$VAR` are **not** expanded; use an absolute path. - **Custom shortcuts** (`inputs/keymap.rs`, issue #211): the optional `[shortcuts]` config table (`action = "Key+Combo"`) remaps the 15 action/navigation keys. `Action`/`KeyBinding`/`Keymap` own all key knowledge; `config.rs` stays a `BTreeMap` and never enumerates the actions. Shortcuts have no CLI flag, so precedence is config.toml > default; `Keymap::from_config(config.shortcuts)` is built in `main` and threaded (by value, no lock) through `app_serial`/`app_rtt` into `InputsConnections`. Config modifier names are **logical** and lowered to the real per-platform crossterm event in `keymap::resolve` — this replaces the old `CTRL_MODIFIER`/`ACTION_MODIFIER` consts in `handle_key_input` (only `ACTION_MODIFIER`, for the fixed `Alt+Enter` arm, remains). `handle_key_input` resolves a remappable action via `private.keymap.action_for()` **before** the intrinsic key match (`try_run_action`); a `Tab`-bound `next_bookmark` falls through to the `@tag` autocomplete arm while the pop-up is up. Text-editing/intrinsic keys (typing, Enter, Esc, arrows, Home/End, Backspace/Delete, headless `Ctrl+K`/`Ctrl+Q`) stay hardcoded and are rejected as override targets (`reserved_reason`). Unknown actions, bad key strings, reserved keys and duplicate bindings are fatal config errors. An unbound `Ctrl`/`Alt`+letter is now swallowed rather than typed literally. - `Ble` is declared as a subcommand but is not implemented (returns an error). -- **Release & installers** (issue #229): distribution is driven by [cargo-dist](https://opensource.axo.dev/cargo-dist/) — config in `dist-workspace.toml` (`[dist]`), the `[profile.dist]` profile and `[package.metadata.wix]` GUIDs in `Cargo.toml`. `.github/workflows/release.yml` is **dist-generated** (do not hand-edit; regenerate with `dist init && dist generate`) and fires on a `vX.Y.Z` tag, building tarballs/zip + shell/powershell installers + a Windows `.msi`. Publishing to crates.io is *not* done by dist — `.github/workflows/publish-crates.yml` handles that on the same tag (OIDC). Maintainer release flow: bump the version, push the `vX.Y.Z` tag. PRs only run `dist plan`; to test the `.msi` before a tag, temporarily set `pr-run-mode = "upload"`, `dist generate` and push — the PR then builds the full artifact set and attaches it to the Actions run without publishing (`gh run download `) — then revert it. `wix/main.wxs` is hand-edited (dist `allow-dirty = ["msi"]`) to add the four per-command Start-Menu shortcuts + their icons; `libudev-dev` is installed on the Linux runner via `[dist.dependencies.apt]`. +- **Release & installers** (issue #229): distribution is driven by [cargo-dist](https://opensource.axo.dev/cargo-dist/) — config in `dist-workspace.toml` (`[dist]`), the `[profile.dist]` profile and `[package.metadata.wix]` GUIDs in `Cargo.toml`. `.github/workflows/release.yml` is **dist-generated** (do not hand-edit; regenerate with `dist init && dist generate`) and fires on a `vX.Y.Z` tag, building tarballs/zip + shell/powershell installers + a Windows `.msi`. Publishing to crates.io is *not* done by dist — `.github/workflows/publish-crates.yml` handles that on the same tag (OIDC). Maintainer release flow: open a PR bumping the version, label it `release`, merge it — `release-tag.yml` creates the tag from that merge (see the next bullet), so there is no manual tagging step. PRs only run `dist plan`; to test the `.msi` before a tag, temporarily set `pr-run-mode = "upload"`, `dist generate` and push — the PR then builds the full artifact set and attaches it to the Actions run without publishing (`gh run download `) — then revert it. `wix/main.wxs` is hand-edited (dist `allow-dirty = ["msi"]`) to add the four per-command Start-Menu shortcuts + their icons; `libudev-dev` is installed on the Linux runner via `[dist.dependencies.apt]`. - **Release security** (defense-in-depth so no PR/non-owner can cut a release): a release only fires on a `vX.Y.Z` tag; the `release-tags` repo ruleset restricts creating tags to admins; `publish-crates.yml` is owner-guarded (`github.actor == github.repository_owner`), goes through the reviewer-gated `crates` environment, and checks tag==Cargo.toml version; `main` requires code-owner review (`.github/CODEOWNERS`) for release-critical paths. `tests/release_security.rs` pins these workflow invariants and fails CI if they regress — keep it green and do not weaken it. Repo-settings pieces (rulesets, environment reviewers) live in GitHub, not the tree. +- **Tagging from the release PR** (`.github/workflows/release-tag.yml`): the tag is not pushed by hand — merging a version bump into `main` cuts it, so the release is a single maintainer-gated flow. Non-obvious constraints: (1) it triggers on **`push`**, never `pull_request` — a `push` run always takes the workflow definition from `main`, whereas a `pull_request` run would use the *PR branch's* copy, letting a PR rewrite the file that holds the tagging token; (2) the tag is pushed with a maintainer PAT (`RELEASE_TAG_TOKEN`, from the `release-tag` environment) because a `GITHUB_TOKEN` push **does not trigger other workflows** (`release.yml`/`publish-crates.yml` would never fire) and tag creation is admin-only per the ruleset — the owner's PAT also keeps `github.actor` on the tag push equal to the owner, so `publish-crates.yml`'s owner guard still holds. Guards: owner-only pusher, the merged PR must carry the `release` label (the same one `version-guard` needs for the bump, making it the single release opt-in), the version must actually have changed (`paths: Cargo.toml` also fires for dependency edits), and an existing tag is never moved. Its `GITHUB_TOKEN` stays `contents: read`. Four invariants are pinned in `tests/release_security.rs`. PAT setup lives in CONTRIBUTING.md → *Release security → One-time setup for automatic tagging*. - **Windows icons** (issue #230): `installer/icons/*.ico` (one base + four per-command variants) are generated by the standalone helper `installer/gen-icons/` (its own workspace, so `image`/`resvg`/`ico` stay out of the scope build) from `imgs/scope-logo.png` + Font Awesome glyphs. `build.rs` embeds `scope.ico` into `scope.exe` on Windows (`winresource`, a `cfg(windows)` build-dep; no-op elsewhere). ## Architecture diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 07d0968..ccb0ea5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -222,6 +222,15 @@ pull request (and no non-owner collaborator) can trigger one: - **Only admins can create tags.** The `release-tags` repository ruleset restricts creating/updating/deleting *any* tag to admins, so a collaborator with push access cannot push a `v*` tag to start a release. +- **The tag comes from the release PR, not from a manual push.** + `release-tag.yml` tags `vX.Y.Z` when a version bump lands on `main`, so the + release is one maintainer-gated flow (review + merge) instead of a merge plus + a remembered `git tag && git push`. It refuses to tag unless the pusher is the + repository owner, the commit belongs to a merged PR labelled `release`, and the + version actually changed; it never moves an existing tag, and it publishes + nothing itself. It triggers on `push` rather than `pull_request` on purpose: + a `push` run always uses the workflow definition from `main`, so a pull + request branch cannot edit the file that holds the tagging token. - **crates.io publishing is reviewer-gated.** `publish-crates.yml` deploys through the `crates` environment (required reviewer: the owner) and is additionally guarded by `if: github.actor == github.repository_owner`; it also @@ -233,8 +242,31 @@ pull request (and no non-owner collaborator) can trigger one: security tests). `tests/release_security.rs` parses the workflows and fails CI if any of these invariants regress. -Maintainer release flow: bump the version in `Cargo.toml`, then push the -matching `vX.Y.Z` tag (approve the `crates` deployment when prompted). +Maintainer release flow: open a PR that bumps `version` in `Cargo.toml`, label it +`release`, and merge it once CI is green. The merge tags `vX.Y.Z`, which builds +the binaries and installers and publishes to crates.io (approve the `crates` +deployment when prompted). No manual tagging step. + +### One-time setup for automatic tagging + +`release-tag.yml` pushes the tag with a maintainer PAT, exposed as the +`RELEASE_TAG_TOKEN` secret of a `release-tag` environment. A PAT is required +rather than the built-in `GITHUB_TOKEN` for two reasons: a `GITHUB_TOKEN` push +**does not trigger further workflow runs**, so `release.yml` would never fire; +and tag creation is admin-only per the `release-tags` ruleset. Using the owner's +PAT also keeps `github.actor` on the tag push equal to the owner, so +`publish-crates.yml`'s owner guard still applies. + +1. Create a **fine-grained** PAT owned by the repository owner, scoped to this + repository only, with `Contents: read and write` — nothing else — and the + shortest expiry you're willing to rotate. +2. Create a `release-tag` environment (*Settings → Environments*) and add the PAT + as the secret `RELEASE_TAG_TOKEN`. Limit its deployment branches to `main`. + Adding a required reviewer there is optional; it gives you a second OK before + any release goes out, at the cost of one approval click. + +Rotate the PAT when it expires. Without it the tagging job fails loudly with a +pointer to this section rather than silently skipping a release. ## Testing the installers before a release diff --git a/tests/release_security.rs b/tests/release_security.rs index b662c6f..f91386a 100644 --- a/tests/release_security.rs +++ b/tests/release_security.rs @@ -14,6 +14,11 @@ //! * The dist-generated `release.yml` never publishes on a pull request and //! isn't triggered by a branch push. //! * `version-guard.yml` still runs on pull requests. +//! * `release-tag.yml` — which turns a merged release PR into the `vX.Y.Z` +//! tag — is triggered by `push` and never by `pull_request` (so its own +//! definition always comes from `main`, out of reach of a PR branch), is +//! guarded to the repository owner, requires the merged PR's `release` +//! label, and holds a read-only `GITHUB_TOKEN`. //! //! These are defense-in-depth: the root protections (tag protection ruleset, //! branch protection on `main`, the `crates` environment reviewers) live in the @@ -179,6 +184,79 @@ fn release_never_publishes_on_pull_request() { ); } +#[test] +fn release_tag_is_never_driven_by_a_pull_request() { + let (_text, wf) = read_workflow("release-tag.yml"); + let on = on_block(&wf); + + // The whole point of the `push` trigger: for a push, GitHub takes the + // workflow definition from the pushed branch (`main`), so a PR branch can't + // rewrite this file and still get the tag-pushing token. A + // `pull_request`-triggered run would use the PR's copy of the definition. + assert!( + !has_key(on, "pull_request") && !has_key(on, "pull_request_target"), + "release-tag must not be triggered by a pull request — its definition \ + would then come from the PR branch, which can edit it" + ); + + let push = get(on, "push").expect("release-tag triggers on push"); + let branches = get(push, "branches") + .and_then(Value::as_sequence) + .expect("release-tag push.branches is a list"); + assert!( + branches.iter().filter_map(Value::as_str).eq(["main"]), + "release-tag must only tag pushes to main, got {branches:?}" + ); +} + +#[test] +fn release_tag_is_owner_guarded_and_label_gated() { + let (_text, wf) = read_workflow("release-tag.yml"); + + // The workflow's own token must not be able to write anything; the tag is + // pushed with the maintainer PAT instead. + assert_eq!( + get(&wf, "permissions") + .and_then(|p| get(p, "contents")) + .and_then(Value::as_str), + Some("read"), + "release-tag's GITHUB_TOKEN must stay read-only for contents" + ); + + let tag = get(&wf, "jobs") + .and_then(|jobs| get(jobs, "tag")) + .expect("release-tag has a `tag` job"); + + let guard = get(tag, "if") + .and_then(Value::as_str) + .expect("tag job has an `if:` guard"); + assert!( + guard.contains("github.actor") && guard.contains("github.repository_owner"), + "tag job must be guarded to the repo owner; got if: {guard:?}" + ); + + assert_eq!( + get(tag, "environment").and_then(Value::as_str), + Some("release-tag"), + "tag job must draw its PAT from the `release-tag` environment" + ); + + // The tag is only cut for a merged PR that carries the `release` label — + // the same label version-guard demands before allowing the bump. + let steps = get(tag, "steps") + .and_then(Value::as_sequence) + .expect("tag job has steps"); + let checks_label = steps.iter().any(|step| { + get(step, "run") + .and_then(Value::as_str) + .is_some_and(|run| run.contains("/pulls") && run.contains("'release'")) + }); + assert!( + checks_label, + "tag job must require the merged PR to carry the `release` label" + ); +} + #[test] fn version_guard_runs_on_pull_requests() { let (_text, wf) = read_workflow("version-guard.yml");