Skip to content

BUILD-12473 Pin ci-github-actions consistently to master, grant contents:write - #117

Merged
julien-carsique-sonarsource merged 1 commit into
masterfrom
chore/jcarsique/BUILD-12473-preCommitContentsWrite
Sep 9, 2026
Merged

julien-carsique-sonarsource merged 1 commit into
masterfrom
chore/jcarsique/BUILD-12473-preCommitContentsWrite

Conversation

@julien-carsique-sonarsource

@julien-carsique-sonarsource julien-carsique-sonarsource commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

This repo dogfoods ci-github-actions@master. Master now requires contents: write for get-build-number and everything that calls it internally (including config-npm), per PREQ-7781.

Two changes:

  1. Adds contents: write to the pre-commit job in pre-commit.yml, which calls config-npm@master. Mirrors the fix already merged in sonar-dummy-maven-enterprise#143.
  2. Pins config-pip to @master (was @v1) in both build.yml and pre-commit.yml, consistent with the sibling actions in those same jobs (build-poetry@master, promote@master, config-npm@master).

(2) isn't cosmetic: mixing config-pip@v1 with @master actions in the same job breaks BUILD_NUMBER propagation. config-pip@v1's get-build-number@v1 unconditionally exports BUILD_NUMBER via GITHUB_ENV; the v2 (@master) get-build-number then sees that env var already set and skips its own ref-claiming step — so contents: write was never actually being exercised by this job. Worse, downstream @master steps expect the v2 numbering scheme and choke on the v1-style value: see master run 34352891924, where the Promote job fails with Build number '<value>' is not a valid positive integer.

Tracked in BUILD-12473.

FYI: #116 rewrites the pre-commit job's steps and would incidentally remove the config-npm@master call, but it doesn't touch the permissions: block, so the two remain compatible in either merge order.


Summary by Gitar

  • Workflow updates:
    • Pinned config-pip to @master and granted contents: write permissions in slack_notify.yml

This will update automatically on new commits.

@julien-carsique-sonarsource
julien-carsique-sonarsource requested a review from a team as a code owner September 9, 2026 09:04
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

BUILD-12473

Comment thread .github/workflows/pre-commit.yml
@julien-carsique-sonarsource

Copy link
Copy Markdown
Contributor Author

Close in favor of #116

@julien-carsique-sonarsource julien-carsique-sonarsource changed the title BUILD-12473 Grant contents:write to pre-commit job BUILD-12473 Pin ci-github-actions consistently to master, grant contents:write Sep 9, 2026
Comment thread .github/workflows/build.yml
…nts:write

This repo dogfoods ci-github-actions@master. Master now requires
contents: write for get-build-number and everything that calls it
internally (including config-npm), per PREQ-7781.

- Adds contents: write to the pre-commit job in pre-commit.yml,
  which calls config-npm@master.
- Pins config-pip to @master (was @v1) in build.yml, pre-commit.yml,
  and slack_notify.yml, consistent with the sibling actions in those
  jobs (build-poetry@master, promote@master, config-npm@master).

(2) isn't cosmetic: mixing config-pip@v1 with @master actions in the
same job breaks BUILD_NUMBER propagation. config-pip@v1's
get-build-number@v1 unconditionally exports BUILD_NUMBER via
GITHUB_ENV; the v2 (@master) get-build-number then sees that env var
already set and skips its own ref-claiming step — so contents: write
was never actually being exercised by this job. Worse, downstream
@master steps expect the v2 numbering scheme and choke on the
v1-style value: see master run 34352891924, where the Promote job
fails with "Build number '<value>' is not a valid positive integer".

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@julien-carsique-sonarsource
julien-carsique-sonarsource force-pushed the chore/jcarsique/BUILD-12473-preCommitContentsWrite branch from 401252f to 47fb831 Compare September 9, 2026 14:11
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@julien-carsique-sonarsource
julien-carsique-sonarsource merged commit 1244704 into master Sep 9, 2026
10 checks passed
@julien-carsique-sonarsource
julien-carsique-sonarsource deleted the chore/jcarsique/BUILD-12473-preCommitContentsWrite branch September 9, 2026 15:22
@gitar-bot

gitar-bot Bot commented Oct 13, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 closed / 2 findings

Pins config-pip to @master and grants contents: write to the pre-commit job, fixing BUILD_NUMBER propagation when mixing v1 and v2 actions. Also updates slack_notify.yml consistently.

✅ 2 closed
✅ Quality: config-pip left on @v1 in slack_notify.yml; job lacks contents: write

📄 .github/workflows/build.yml:27 📄 .github/workflows/pre-commit.yml:17
This commit pins config-pip to @master # dogfood in build.yml and pre-commit.yml, but .github/workflows/slack_notify.yml:18 still uses SonarSource/ci-github-actions/config-pip@v1, so the "pin config-pip to master consistently with sibling dogfood actions" change is partial (every other ci-github-actions reference in the repo is @master). Note that config-pip invokes ./.actions/get-build-number internally on both refs, and this workflow only grants workflow-level contents: read — so pinning that step to master (or the floating v1 tag picking up the PREQ-7781 change) makes the notify job fail the same way pre-commit did, unless contents: write is added to it. Either pin it and grant the permission, or state explicitly why this site stays on v1.

Closed: Quality: contents: write likely not exercised in this job (BUILD_NUMBER preset)

📄 .github/workflows/pre-commit.yml:9-11 📄 .github/workflows/pre-commit.yml:17 📄 .github/workflows/pre-commit.yml:31 🔗 master: from-env skip gate + git/refs claim 🔗 v1: exports BUILD_NUMBER to GITHUB_ENV, uses Vault token
In this job config-pip@v1 (line 17) runs before config-npm@master (line 31), and get-build-number@v1 unconditionally exports BUILD_NUMBER to $GITHUB_ENV (v1 get-build-number/action.yml "Export build number" step). get-build-number@master — the one config-npm@master symlinks and invokes — gates its ref-claiming step on steps.from-env.outputs.skip != 'true', and from-env fires when env.BUILD_NUMBER != '', so the POST /repos/.../git/refs call that needs contents: write is skipped in this workflow; the v1 claim path uses the Vault-provided token against the custom-properties API and only needs id-token: write. Since this workflow is pull_request-triggered and runs pre-commit hooks resolved from the PR branch's .pre-commit-config.yaml, the widened token is worth confirming against an actual failing run before keeping it; if the job was failing, the failure is not the ref-claim path and the fix may be masking a different cause (and it can go back to contents: read once #116 removes the config-npm@master step).

Verification

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled

Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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