Skip to content

ci: scope workflow write tokens to the job, not top level - #251

Merged
jensholdgaard merged 2 commits into
mainfrom
ci/scorecard-token-permissions-job-scope
Jun 18, 2026
Merged

jensholdgaard merged 2 commits into
mainfrom
ci/scorecard-token-permissions-job-scope

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 18, 2026

Copy link
Copy Markdown
Owner

What

Moves contents: write from the workflow top level to the single job that needs it in release.yml and capture-otel-demo-corpus.yml, and sets the top level to contents: read.

Why

OpenSSF Scorecard's Token-Permissions check currently scores 0/10 — it flags top-level write permissions because a compromised step in any job of that workflow inherits write access to the repo. Least privilege is top-level read, escalating to write only on the job that needs it. ci.yml already follows this pattern (top-level read, job-level write on the docs-publish job); this brings the remaining two workflows in line.

  • release.ymlcontents: write now lives on the changelog job (where action-gh-release creates the Release).
  • capture-otel-demo-corpus.ymlcontents: write now lives on the capture job (where the optional release_tag publish step runs).

Risk

None functional — the jobs that need write still get it; the top-level default drops to read. No invariant (CLAUDE.md §3) is touched; this is CI-hygiene only.

Scorecard context

Part of a triage of the 5.3 overall score. This is the unambiguous, in-repo, zero-risk win. Other low checks are either policy-deferred (Fuzzing/Signed-Releases → shipping milestone), self-healing (Maintained → repo age), external (CII badge, multi-org Contributors), settings-level (Branch-Protection needs a PAT + rules), or accepted (Code-Review admin-squash artifact; the 2 OSV Vulnerabilities are reviewed transitive unmaintained-crate advisories tracked in deny.toml).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Enhanced CI/CD workflow security by reducing default permissions and scoping elevated access to only the jobs that require it.

Scorecard's Token-Permissions check penalizes top-level `write`
permissions (a compromised step in any job inherits them). `release.yml`
and `capture-otel-demo-corpus.yml` declared `contents: write` at the
workflow top level; move it to the single job that needs it (the release
/ corpus-publish step) and set the top level to `contents: read`,
matching the pattern already used in `ci.yml`.

No behavior change — the same job still gets write; every other job
(none here, but future additions) now defaults to read.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 52 minutes and 50 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 886c0b3b-460b-4b47-9e82-920acffceca0

📥 Commits

Reviewing files that changed from the base of the PR and between e9b58b4 and f89a64c.

📒 Files selected for processing (1)
  • .github/workflows/capture-otel-demo-corpus.yml
📝 Walkthrough

Walkthrough

Two GitHub Actions workflow files have their permissions.contents setting moved from write at the workflow level to read at the workflow level, with write re-granted only at the job level for the specific jobs that require it (capture and changelog).

Changes

GitHub Actions Permission Scoping

Layer / File(s) Summary
Scope contents: write to specific jobs
.github/workflows/capture-otel-demo-corpus.yml, .github/workflows/release.yml
Both workflows change the top-level permissions.contents from write to read. The capture job in capture-otel-demo-corpus.yml and the changelog job in release.yml each gain their own permissions.contents: write block to retain the ability to publish release assets.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • jensholdgaard/ourios#73: Originally introduced the capture-otel-demo-corpus.yml workflow whose permissions are being tightened here.
  • jensholdgaard/ourios#79: Previously modified permissions.contents in capture-otel-demo-corpus.yml, directly preceding this adjustment.

Poem

🐇 A rabbit knows to keep keys small,
Don't grant write to one and all!
Each job now holds its own small right,
The changelog writes, the rest stay light.
Least privilege hops through the night! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description provides comprehensive context (What, Why, Risk, Scorecard context) but deviates from the repository's template structure which specifies Summary, Related, and Checklist sections. Consider restructuring to follow the template's Summary/Related/Checklist format, or confirm the repository allows alternative description structures for CI-focused changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: moving workflow write tokens from top level to job scope for least-privilege access.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/scorecard-token-permissions-job-scope

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI 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.

Pull request overview

This PR improves GitHub Actions token least-privilege by changing two workflows to default to contents: read at the workflow level and granting contents: write only to the job that actually publishes a GitHub Release / release assets. This aligns these workflows with the existing pattern already used in ci.yml, and should improve OpenSSF Scorecard’s Token-Permissions check.

Changes:

  • .github/workflows/release.yml: move contents: write from workflow-level to the changelog job; set workflow-level to contents: read.
  • .github/workflows/capture-otel-demo-corpus.yml: move contents: write from workflow-level to the capture job; set workflow-level to contents: read.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/release.yml Sets workflow default token permissions to read-only; escalates to contents: write only for the release-creating job.
.github/workflows/capture-otel-demo-corpus.yml Sets workflow default token permissions to read-only; escalates to contents: write only for the job that may publish a corpus release.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/capture-otel-demo-corpus.yml Outdated
Per Copilot review on #251: granting the capture job `contents: write`
still exposes a write-capable token to the ~20 third-party demo
container images it runs for hours. Split the release publish into a
separate `publish` job (`needs: capture`, gated on `release_tag`)
that downloads the captured artifact and cuts the release — it runs no
third-party code and is the only job holding `contents: write`. The
capture job drops to `contents: read`.

download-artifact is SHA-pinned (v4.3.0); the publish job reconstructs
the notes inputs (demo_ref, locust_users, duration, failure_flags) it
previously read from the capture job's env.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@jensholdgaard
jensholdgaard merged commit 57aba55 into main Jun 18, 2026
14 checks passed
@jensholdgaard
jensholdgaard deleted the ci/scorecard-token-permissions-job-scope branch June 18, 2026 23:01
jensholdgaard added a commit that referenced this pull request Jun 19, 2026
… #refs

Per Copilot review:
- The miner_roundtrip oracle was ambiguous across body kinds — a
  structured body renders Faithful via the §6.1 canonical encoding, not
  from a raw 'original body'. Scope the target explicitly to String
  bodies (the Drain template + line-reconstruction path §3.3 governs);
  note structured-body canonical round-trip as a separate candidate
  target (§7). Updated §3.2 table + prose and §5 RFC0015.1.
- '#251' doesn't autolink in mdBook (GitHub-UI convenience); replace the
  two references with prose ('the workflow-token least-privilege
  pattern the other workflows follow').

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 19, 2026
* docs(rfc-0015): add fuzzing-harness RFC (specified)

cargo-fuzz/libFuzzer targets on the highest-risk surfaces — the
template miner (asserting the §3.3 bit-identical-reconstruction
invariant, not just panics) and the untrusted-input parsers (OTLP
protobuf/JSON, WAL frame). Phased CI: a bounded smoke-fuzz job now,
ClusterFuzzLite (continuous fuzzing + corpus persistence + Scorecard
detection) as a follow-up. The new fuzz/ workspace member is the §7
new-crate commitment this RFC records; nightly is contained to the
fuzz member, leaving the §6.1 stable pin intact for shipping crates.

Status: specified — §5 acceptance scenarios (RFC0015.1–.7) and §6
testing strategy complete; lands at the specified gate per
docs/rfcs/README.md. Implementation follows in slices (red → green).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): correct unsafe-waiver wording (no existing precedent)

Per Copilot review: the RFC implied ourios-parquet already carries an
unsafe exemption. It does not — every crate root incl. ourios-parquet
is #![deny(unsafe_code)] and the workspace sets unsafe_code = "deny".
Reword §3.1 + §7 to describe the fuzz/ member opting out of the
workspace lint as the *first* such RFC-justified waiver (which §6.1
permits), citing ourios-parquet only as §6.1's hypothetical example.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): use real ourios-miner/wal type & signature names

Per Copilot review: the RFC referenced a nonexistent
`Reconstruction::Lossy` variant — the enum is `Faithful` /
`RetainedVerbatim` (reconstruct.rs:162). Restate the miner oracle as:
`Faithful` ⇒ bytes equal the ingested body, else `RetainedVerbatim`
(body retained verbatim — §3.3's escape hatch). Also fix the loose
`read_frame(Cursor<&[u8]>)` notation to the real
`read_frame(&mut Cursor::new(data))` (`fn read_frame<R: Read>(r: &mut R)`).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): scope miner oracle to string bodies; drop GitHub-only #refs

Per Copilot review:
- The miner_roundtrip oracle was ambiguous across body kinds — a
  structured body renders Faithful via the §6.1 canonical encoding, not
  from a raw 'original body'. Scope the target explicitly to String
  bodies (the Drain template + line-reconstruction path §3.3 governs);
  note structured-body canonical round-trip as a separate candidate
  target (§7). Updated §3.2 table + prose and §5 RFC0015.1.
- '#251' doesn't autolink in mdBook (GitHub-UI convenience); replace the
  two references with prose ('the workflow-token least-privilege
  pattern the other workflows follow').

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): address maintainer review — oracle, resolved §7, §4 depth

Maintainer sign-off review (LGTM, land at specified once clarified):
- RFC0015.1 + §3.2: make explicit that for a string-body record the
  rendered bytes equal the original body in BOTH outcomes — Faithful
  (rebuilt) and RetainedVerbatim (retained verbatim); a retention
  failure is as much a §3.3 violation as a rebuild mismatch.
- §3.4: run all four targets in the smoke job (parsers are cheap); add
  a concrete budget example (~60 s PR / ~300 s schedule) + pinned
  nightly invocation.
- §7: record the maintainer's decisions — dated nightly pin, fuzzing
  cargo feature for read_frame exposure (over a doc-hidden shim), 60/300 s
  budgets, hand-written Arbitrary for the string-body OtlpLogRecord.
- §4: deepen the proptest and OSS-Fuzz alternatives per review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): correct miner_roundtrip read-back mechanics

Per Copilot review: `MinerCluster::ingest` returns only a template_id,
not the MinedRecord + tokens `render` needs. Describe the real flow the
existing invariants.rs tests use — attach an observable RecordSink
(SharedRecordSink), ingest, drain the emitted MinedRecord, look up leaf
tokens via `templates_for(tenant)` for the record's
(template_id, template_version), then `render`. Fixed the §3.2 table
entry-point cell + prose and §5 RFC0015.1's When clause.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): note wal_frame exposure in table; soften §7 status wording

Per Copilot review:
- wal_frame table cell: read_frame is pub(crate) today; note it's exposed
  to the target via the `fuzzing` feature (already covered in §3.2/§7).
- §7: reword 'Resolved (maintainer sign-off)' → 'Maintainer review gave
  direction … planned approach, to be confirmed as the RFC advances',
  since a specified-stage RFC is still in review (not terminal).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0015): fix crate path libfuzzer-sys -> libfuzzer_sys

Per Copilot review: `libfuzzer-sys::fuzz_target!` isn't a valid Rust
path (hyphens aren't allowed in identifiers). The crate is `libfuzzer-sys`
on crates.io but the path identifier is `libfuzzer_sys`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 2, 2026
…path (#336)

* ci(corpus): fix release publishing (GH_REPO) + publish-only recovery path

The corpus/otel-demo-v7 publish failed after its 5-hour capture succeeded:
`gh release create` → "fatal: not a git repository". The publish job has no
checkout (deliberate — #251 split it off so the write token never coexists
with the third-party demo images), so `gh` cannot resolve the repository from
a git remote. v6 predates that split (2026-06-10 vs #251 on 2026-06-19); v7
was the first publish attempt since, hitting the latent gap. Fix: set
`GH_REPO: ${{ github.repository }}` on the publish step.

Also add the recovery path so the successful capture is not wasted: a new
`artifact_run_id` dispatch input skips the capture job and publishes from the
`otel-demo-corpus` artifact of the named earlier run (cross-run
download-artifact with the token; retained until 2026-09-30 for v7's run).
The release notes now record the capture run id, and the input docs require
passing the original capture's parameters so the notes stay truthful.

Verified: YAML parses; actionlint clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(corpus): truthful provenance on the recovery path

Copilot's review of #336, both accepted:

- The artifact_run_id input doc named a non-existent `duration` input;
  corrected to `duration_seconds`.
- On the recovery path, `source commit` recorded the publish dispatch's
  GITHUB_SHA instead of the commit the capture ran at; the publish step now
  resolves the capture run's head_sha via the Actions API when
  CAPTURE_RUN_ID differs from the current run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants