ci(kotlin-sdk): couple Maven Central deploy to the kotlin-sdk-v* release tag (stacked on #4182)#4193
Conversation
📝 WalkthroughWalkthroughThe Kotlin SDK release workflow now builds from an immutable tag commit, prevents tag drift across approval delays, avoids overwriting existing AAR assets, and conditionally deploys Maven artifacts using environment-scoped credentials. Publishing documentation describes the CI and manual release paths. ChangesKotlin SDK release pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseTag
participant BuildJob
participant GitHubRelease
participant MavenDeploy
ReleaseTag->>BuildJob: Resolve tag to commit SHA
BuildJob->>BuildJob: Build and record immutable SHA
BuildJob->>ReleaseTag: Verify tag still resolves to built SHA
BuildJob->>GitHubRelease: Attach sdk-release.aar when absent
MavenDeploy->>ReleaseTag: Revalidate tag after approval
MavenDeploy->>MavenDeploy: Restore libraries and stage signed artifacts
MavenDeploy->>MavenDeploy: Deploy when environment secrets exist
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
⛔ Blockers found — Sonnet deferred (commit 3e83234) |
|
Reviewed (two independent passes): the tag→deploy coupling properly closes the provenance gap — checkout and version both derive from the same |
…l fast Address @shumkov's review on dashpay#4193. - Split the release workflow into build-and-release and a dedicated maven-central-deploy job that declares environment: maven-central, so GitHub's required-reviewer protection gates the irrevocable publish. The deploy job checks out the same tag and re-stages the native libs the build job uploaded (no cargo/NDK rebuild); Central secrets are referenced in that job so they resolve from the environment when scoped there. - Move the missing/partial-secrets precondition check to run right after version derivation, before the multi-hour native build, so a misconfiguration fails fast. The deploy stays gated on the same result via the deploy job's needs/if. - Strip a refs/tags/ prefix before the kotlin-sdk-v* match so a full-ref workflow_dispatch tag input derives the version instead of silently downgrading to AAR-only. Emit the normalized tag as an output and use it for the GitHub release. - PUBLISHING.md: document that the maven-central environment must be created with required reviewers before secrets are installed (the human gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Implemented the human gate you asked for: the Maven Central staging + Round-2 hardening from our own review: the deploy job now checks out the exact SHA the build job resolved (not the mutable tag), so a tag force-moved during the approval pause can't stage new source against the original native libs; and the PUBLISHING secret-scoping wording is corrected (repo/org-scoped names for the presence check, environment-scoped value overrides optional). |
|
Round-3 verification (two independent passes, reconciled): the mechanics are genuinely implemented — environment-gated deploy job, exact-SHA checkout defeating tag force-moves, version via job outputs, guards running in the deploy job, fail-fast secrets precheck, full-ref stripping. But the release trust boundary is not yet fail-closed — three findings:
P2: the jniLibs artifact has |
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The Maven publication path correctly derives versions from release names and pins deployment to the commit that produced the native libraries. Three blocking issues remain: publishing credentials are exposed before environment approval, manual dispatches can publish branch commits without an existing release tag, and the native artifact can expire while deployment waits for approval.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/kotlin-sdk-release.yml`:
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:105-118: Repository-scoped credentials bypass the environment approval boundary
The unprotected `build-and-release` job reads all Maven Central and GPG credentials, and the publishing guide explicitly requires repository/organization-scoped copies so this check can see them. Consequently, environment approval does not protect either the credentials or publication: a workflow executed from a tag pointing to a commit with modified workflow steps can exfiltrate these credentials or invoke publication directly before `maven-central-deploy` enters the protected environment. Keep the credentials environment-scoped and use a non-secret repository variable to decide whether to build/upload the deployment artifact, or validate the credentials only inside an environment-protected job.
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:42-64: Manual releases can publish from a branch instead of an existing tag
The dispatch value is passed directly to `actions/checkout`, which accepts branch names as well as tag names, while version derivation only checks the resulting text. An explicit input such as `kotlin-sdk-v1.2.3` can therefore resolve to a same-named branch, and omitting the input while dispatching from such a branch has the same effect. The workflow then publishes version `1.2.3` from a commit that was not selected through an existing release tag, violating the documented tag/version/commit invariant. Normalize manual inputs to `refs/tags/<name>`, reject dispatches without a tag ref, and verify that the resolved tag commit equals the checked-out `HEAD` before enabling deployment.
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:206-213: Native artifact can expire while deployment awaits approval
The protected deployment job can remain pending for human approval longer than one day, but its required JNI artifact is retained for only one day from upload. Approval after expiration makes `actions/download-artifact` fail, so the release cannot deploy despite the workflow explicitly supporting an extended approval pause. Retain the artifact for at least the expected approval window, such as seven days, or use the repository default retention period.
| # The `secrets` context is mapped into env vars here because emptiness of | ||
| # the secret is what decides the gate. These names must be resolvable at | ||
| # repository/organization scope for this fail-fast check to see them; the | ||
| # human approval gate and (optionally) environment-scoped copies live on | ||
| # the maven-central environment — see packages/kotlin-sdk/PUBLISHING.md. | ||
| - name: Check Maven Central deploy preconditions | ||
| id: maven-gate | ||
| env: | ||
| SDK_VERSION: ${{ steps.sdk-version.outputs.version }} | ||
| PORTAL_USER: ${{ secrets.JRELEASER_MAVENCENTRAL_SONATYPE_USERNAME }} | ||
| PORTAL_TOKEN: ${{ secrets.JRELEASER_MAVENCENTRAL_SONATYPE_PASSWORD }} | ||
| GPG_SECRET_KEY: ${{ secrets.JRELEASER_GPG_SECRET_KEY }} | ||
| GPG_PUBLIC_KEY: ${{ secrets.JRELEASER_GPG_PUBLIC_KEY }} | ||
| GPG_PASSPHRASE: ${{ secrets.JRELEASER_GPG_PASSPHRASE }} |
There was a problem hiding this comment.
🔴 Blocking: Repository-scoped credentials bypass the environment approval boundary
The unprotected build-and-release job reads all Maven Central and GPG credentials, and the publishing guide explicitly requires repository/organization-scoped copies so this check can see them. Consequently, environment approval does not protect either the credentials or publication: a workflow executed from a tag pointing to a commit with modified workflow steps can exfiltrate these credentials or invoke publication directly before maven-central-deploy enters the protected environment. Keep the credentials environment-scoped and use a non-secret repository variable to decide whether to build/upload the deployment artifact, or validate the credentials only inside an environment-protected job.
source: ['codex']
There was a problem hiding this comment.
Resolved in e22c26e — Repository-scoped credentials bypass the environment approval boundary no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| @@ -26,8 +45,95 @@ jobs: | |||
| # A manual dispatch runs on the branch/tag selected in the UI, not | |||
| # on the `tag` input it will publish under — without this ref the | |||
| # released AAR could be built from a different commit than the tag. | |||
| # A full `refs/tags/...` value is a valid checkout ref as-is. | |||
| ref: ${{ github.event.inputs.tag || github.ref }} | |||
|
|
|||
| - name: Resolve built commit SHA | |||
| id: resolve-sha | |||
| run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" | |||
|
|
|||
| # The Maven Central version comes from the tag and ONLY from the tag | |||
| # (kotlin-sdk-vX.Y.Z -> X.Y.Z). Because the checkout above is pinned to | |||
| # the same tag, the GitHub-release AAR and the Maven Central artifact | |||
| # produced by this run are always built from the same commit under the | |||
| # same version — the two channels cannot drift. | |||
| - name: Derive SDK version from release tag | |||
| id: sdk-version | |||
| env: | |||
| TAG: ${{ github.event.inputs.tag || github.ref_name }} | |||
| run: | | |||
There was a problem hiding this comment.
🔴 Blocking: Manual releases can publish from a branch instead of an existing tag
The dispatch value is passed directly to actions/checkout, which accepts branch names as well as tag names, while version derivation only checks the resulting text. An explicit input such as kotlin-sdk-v1.2.3 can therefore resolve to a same-named branch, and omitting the input while dispatching from such a branch has the same effect. The workflow then publishes version 1.2.3 from a commit that was not selected through an existing release tag, violating the documented tag/version/commit invariant. Normalize manual inputs to refs/tags/<name>, reject dispatches without a tag ref, and verify that the resolved tag commit equals the checked-out HEAD before enabling deployment.
source: ['codex']
There was a problem hiding this comment.
Resolved in this update — Manual releases can publish from a branch instead of an existing tag no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - name: Upload native libraries for the deploy job | ||
| if: steps.maven-gate.outputs.deploy == 'true' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: kotlin-sdk-jnilibs | ||
| path: packages/kotlin-sdk/sdk/src/main/jniLibs | ||
| if-no-files-found: error | ||
| retention-days: 1 |
There was a problem hiding this comment.
🔴 Blocking: Native artifact can expire while deployment awaits approval
The protected deployment job can remain pending for human approval longer than one day, but its required JNI artifact is retained for only one day from upload. Approval after expiration makes actions/download-artifact fail, so the release cannot deploy despite the workflow explicitly supporting an extended approval pause. Retain the artifact for at least the expected approval window, such as seven days, or use the repository default retention period.
| - name: Upload native libraries for the deploy job | |
| if: steps.maven-gate.outputs.deploy == 'true' | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: kotlin-sdk-jnilibs | |
| path: packages/kotlin-sdk/sdk/src/main/jniLibs | |
| if-no-files-found: error | |
| retention-days: 1 | |
| retention-days: 7 |
source: ['codex']
There was a problem hiding this comment.
Resolved in e22c26e — Native artifact can expire while deployment awaits approval no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
|
@thepastaclaw all three blocking findings are addressed in 1. Credentials bypassing the environment gate. The unprotected
2. Manual dispatch publishing from a branch. Added a pre-checkout 3. Native artifact expiry. Validation: YAML parses, One behavioral trade-off worth flagging: because the unprotected job can no longer read secrets, the "secrets not configured" case is now detected only after environment approval (the gated job's first step no-ops the publish via |
|
Thanks — I checked
The exact head is already queued for the full automated re-review. The PR is also currently |
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The workflow now moves publishing-secret references behind the protected environment and retains JNI artifacts for 90 days. Three blocking issues remain: the publishing guide still requires unprotected credential copies, manual dispatch can publish from a branch when its optional tag input is omitted, and a force-moved tag can replace the GitHub release asset after the matching Maven version is immutable.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/PUBLISHING.md`:
- [BLOCKING] packages/kotlin-sdk/PUBLISHING.md:52-73: Repository-scoped credentials bypass the environment approval boundary
The workflow now references all five publishing secrets only inside the environment-gated `maven-central-deploy` job, but the guide still requires repository/organization-scoped copies for a build-job precondition that was removed. Those copies remain available to eligible workflow jobs before `maven-central` approval, allowing the Central token or signing key to be consumed or exfiltrated without the required-reviewer gate. Store these credentials exclusively as `maven-central` environment secrets and update the repository-secret wording accordingly.
In `.github/workflows/kotlin-sdk-release.yml`:
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:216-220: A force-moved tag can still split the GitHub and Maven artifacts
The GitHub release asset is attached before the protected deployment job, using the fixed name `sdk-release.aar`, and `softprops/action-gh-release@v2` overwrites a same-named asset by default. After version `1.2.3` from commit A is published immutably to Maven Central, force-moving that tag to commit B can trigger a run that replaces the GitHub AAR with B; Central cannot replace the existing version, and rejecting the later deployment does not restore the asset. Pinning deployment to the current run's SHA only preserves consistency within one run. Reject tag/version reuse for a different SHA and verify the tag still resolves to the built SHA before attaching or deploying, or enforce and document protected tags plus immutable releases.
| 3. Only then install the Central secrets (below). Install the five secrets at | ||
| **repository/organization scope** so the build job's fail-fast precondition | ||
| check can see them (that check runs before the long native build, so a | ||
| misconfiguration aborts early — and it reads secrets at repo/org scope, not | ||
| from the environment). Do **not** scope them environment-only: that would | ||
| make the presence check compute `deploy=false` and silently disable | ||
| publishing (green build, only a `::notice`). The `maven-central` environment | ||
| provides the required-reviewer human gate; if you additionally want the | ||
| secret *values* withheld from the build job, add environment-scoped copies | ||
| that override at deploy time — but the repo/org-scoped names must remain for | ||
| the presence check. | ||
| If a `kotlin-sdk-v*` tag is pushed while no reviewer approves, the deploy job | ||
| simply waits and can be rejected — the GitHub-release AAR is already attached | ||
| by job 1 regardless. | ||
|
|
||
| **Secrets gate:** the Maven deploy steps only run when the Sonatype Central | ||
| Portal credentials are configured as GitHub repository secrets. That Portal | ||
| token is currently a personal token (held by HashEngineering, the `org.dashj` | ||
| publisher), so until it is installed as repo secrets the deploy steps **skip | ||
| with a notice** instead of failing — the tag still produces the | ||
| GitHub-release AAR, and the Maven release can be done manually with the | ||
| runbook below, **from the tag's commit with the tag's version**. Required |
There was a problem hiding this comment.
🔴 Blocking: Repository-scoped credentials bypass the environment approval boundary
The workflow now references all five publishing secrets only inside the environment-gated maven-central-deploy job, but the guide still requires repository/organization-scoped copies for a build-job precondition that was removed. Those copies remain available to eligible workflow jobs before maven-central approval, allowing the Central token or signing key to be consumed or exfiltrated without the required-reviewer gate. Store these credentials exclusively as maven-central environment secrets and update the repository-secret wording accordingly.
| 3. Only then install the Central secrets (below). Install the five secrets at | |
| **repository/organization scope** so the build job's fail-fast precondition | |
| check can see them (that check runs before the long native build, so a | |
| misconfiguration aborts early — and it reads secrets at repo/org scope, not | |
| from the environment). Do **not** scope them environment-only: that would | |
| make the presence check compute `deploy=false` and silently disable | |
| publishing (green build, only a `::notice`). The `maven-central` environment | |
| provides the required-reviewer human gate; if you additionally want the | |
| secret *values* withheld from the build job, add environment-scoped copies | |
| that override at deploy time — but the repo/org-scoped names must remain for | |
| the presence check. | |
| If a `kotlin-sdk-v*` tag is pushed while no reviewer approves, the deploy job | |
| simply waits and can be rejected — the GitHub-release AAR is already attached | |
| by job 1 regardless. | |
| **Secrets gate:** the Maven deploy steps only run when the Sonatype Central | |
| Portal credentials are configured as GitHub repository secrets. That Portal | |
| token is currently a personal token (held by HashEngineering, the `org.dashj` | |
| publisher), so until it is installed as repo secrets the deploy steps **skip | |
| with a notice** instead of failing — the tag still produces the | |
| GitHub-release AAR, and the Maven release can be done manually with the | |
| runbook below, **from the tag's commit with the tag's version**. Required | |
| 3. Only then install the five Central/GPG secrets listed below as | |
| **environment secrets on `maven-central`**. Do not create repository- or | |
| organization-scoped copies: those scopes would make the credentials | |
| available to jobs that have not passed this environment's reviewer gate. | |
| If a `kotlin-sdk-v*` tag is pushed while no reviewer approves, the deploy job | |
| simply waits and can be rejected — the GitHub-release AAR is already attached | |
| by job 1 regardless. | |
| **Secrets gate:** the Maven deploy steps only run when the Sonatype Central | |
| Portal credentials are configured as secrets on the **`maven-central` | |
| environment**. That Portal token is currently a personal token (held by | |
| HashEngineering, the `org.dashj` publisher), so until it is installed there | |
| the deploy steps **skip with a notice** instead of failing — the tag still | |
| produces the GitHub-release AAR, and the Maven release can be done manually | |
| with the runbook below, **from the tag's commit with the tag's version**. |
source: ['codex']
There was a problem hiding this comment.
Resolved in this update — Repository-scoped credentials bypass the environment approval boundary no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - name: Attach AAR to release | ||
| uses: softprops/action-gh-release@v2 | ||
| with: | ||
| tag_name: ${{ github.event.inputs.tag || github.ref_name }} | ||
| tag_name: ${{ steps.sdk-version.outputs.tag_name }} | ||
| files: packages/kotlin-sdk/sdk/build/outputs/aar/sdk-release.aar |
There was a problem hiding this comment.
🔴 Blocking: A force-moved tag can still split the GitHub and Maven artifacts
The GitHub release asset is attached before the protected deployment job, using the fixed name sdk-release.aar, and softprops/action-gh-release@v2 overwrites a same-named asset by default. After version 1.2.3 from commit A is published immutably to Maven Central, force-moving that tag to commit B can trigger a run that replaces the GitHub AAR with B; Central cannot replace the existing version, and rejecting the later deployment does not restore the asset. Pinning deployment to the current run's SHA only preserves consistency within one run. Reject tag/version reuse for a different SHA and verify the tag still resolves to the built SHA before attaching or deploying, or enforce and document protected tags plus immutable releases.
source: ['codex']
The manual Maven publish took -PsdkVersion by hand, so nothing tied it to the kotlin-sdk-* tag that drives the GitHub-release AAR — the two channels could ship different commits under one version (raised by shumkov on dashpay#4182). Fold jreleaserDeploy into kotlin-sdk-release.yml: - The Maven version is DERIVED from the tag (kotlin-sdk-vX.Y.Z -> X.Y.Z); there is no version input anywhere in the workflow, and the checkout is already pinned to the same tag, so the Maven Central artifact and the GitHub-release AAR are always the same commit + version. Malformed kotlin-sdk-v* tags and tag-derived -SNAPSHOT versions fail the run; non-versioned kotlin-sdk-* tags keep their AAR-only behavior. - Deploy steps are gated on the Sonatype Central Portal secrets being configured (the token is currently personal): absent secrets skip with a notice, a partial secret set fails rather than guessing. - The existing publish guards (cleanStagingDeploy, verifyJniLibsForRemotePublish, verifyStagedAarForRemotePublish) run in the CI path via the same task dependencies (verified with --dry-run). - workflow_dispatch stays for re-runs/emergencies and is also tag-derived (existing-tag input only). - PUBLISHING.md: document the CI path as preferred and mark the manual runbook as the fallback, pinned to the tag's commit/version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l secrets Review round 2: line-oriented grep let a multiline workflow_dispatch tag input smuggle extra GITHUB_OUTPUT lines past validation (bash [[ =~ ]] matches the whole string, rejecting newlines), and the deploy gate only checked 3 of the 5 secrets the staging/jreleaser steps consume — a missing GPG passphrase or public key now fails at the gate instead of mid-deploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l fast Address @shumkov's review on dashpay#4193. - Split the release workflow into build-and-release and a dedicated maven-central-deploy job that declares environment: maven-central, so GitHub's required-reviewer protection gates the irrevocable publish. The deploy job checks out the same tag and re-stages the native libs the build job uploaded (no cargo/NDK rebuild); Central secrets are referenced in that job so they resolve from the environment when scoped there. - Move the missing/partial-secrets precondition check to run right after version derivation, before the multi-hour native build, so a misconfiguration fails fast. The deploy stays gated on the same result via the deploy job's needs/if. - Strip a refs/tags/ prefix before the kotlin-sdk-v* match so a full-ref workflow_dispatch tag input derives the version instead of silently downgrading to AAR-only. Emit the normalized tag as an output and use it for the GitHub release. - PUBLISHING.md: document that the maven-central environment must be created with required reviewers before secrets are installed (the human gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…secret-scope wording Review round 2: - F1: the deploy job re-checked-out the tag ref, so a tag force-moved during the human-approval pause could stage new source against the original native libs. The build job now outputs its resolved HEAD SHA and the deploy job checks out that SHA, not the mutable tag. - F2: PUBLISHING.md said to scope the Central secrets to the environment 'rather than repo-wide', contradicting the fail-fast presence check which reads them at repo/org scope (environment-only scoping would silently disable publishing). Reworded to require repo/org-scoped names, with environment-scoped value overrides optional. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ironment, require a tag for manual dispatch, extend JNI artifact retention Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ag for empty dispatch; guard against force-moved tags Addresses thepastaclaw review blockers on dashpay#4193: - PUBLISHING.md: step 3 and the Secrets gate now direct admins to install the five JRELEASER_* secrets as environment secrets on `maven-central` only (no repo/org-scoped copies), matching the workflow's gated-job design — the build job no longer reads any publishing secret. - kotlin-sdk-release.yml (release-ref resolution): a workflow_dispatch with an empty `tag` input no longer falls through to the tag-push path. It is accepted only when the run was dispatched at a tag ref (github.ref = refs/tags/...); a branch dispatch without an explicit tag is now rejected, so a manual run can never publish a Maven version derived from a moving branch. - kotlin-sdk-release.yml (force-moved tag guard): before attaching the AAR, hard- fail if the tag no longer resolves to the exact commit this run built, and skip the attach (never overwrite) when an sdk-release.aar is already attached for the tag — so a force-moved tag cannot replace the GitHub AAR of a version already published immutably to Maven Central. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…; scope org-secret removal; verify from tag ref Addresses thepastaclaw review on dashpay#4220: - Blocker (linked guide contradiction): the runbook now reads consistently with the env-only scoping in dashpay#4193's PUBLISHING.md (fixed in tandem) — publishing secrets live only on the `maven-central` environment and this repository's access to any broad copies is removed. - Suggestion (shared org secrets): step 3 no longer instructs unconditional deletion of organization-scoped secrets. It deletes the repo-scoped copies and removes `dashpay/platform`'s access to org copies; an org secret is deleted outright only when no other repository uses it. The ticket summary is aligned. - Suggestion (dispatch from tag ref): the Verify step now dispatches the workflow at the tag ref (gh workflow run --ref kotlin-sdk-vX.Y.Z), noting that supplying only the `tag` input while dispatching from a branch does not satisfy a tag-only environment deployment policy (GitHub evaluates the policy against the run's github.ref). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e22c26e to
6bf611a
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The environment-only secret guidance and omitted-input branch path now resolve two of the three prior blockers. Release provenance remains unsafe across mutable-tag timing windows: checkout can build a different commit than the triggering event, an existing AAR is trusted without provenance, and the tag is not revalidated after environment approval; mutable action references also warrant hardening in the credential-bearing job.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking | 🟡 1 suggestion(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/kotlin-sdk-release.yml`:
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:101-105: Mutable tag checkout can substitute code behind the environment approval
The resolver validates a tag name but passes the mutable tag ref to `actions/checkout`. For a tag-push event created at commit A, a writer can force-move the tag to commit B before checkout; checkout then builds B, and the later tag guard also observes B, so it passes. The protected deployment consequently checks out B and executes B's Gradle wrapper with the signing and Maven credentials even though the workflow run and pending environment approval originated from the push of A. Resolve an immutable commit before checkout: use the event SHA for tag pushes, peel the selected tag to a commit SHA for manual dispatches, checkout that SHA, and retain the later tag-to-SHA guards.
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:352-361: Revalidate the release tag after the approval delay
The only tag-to-built-SHA check runs in `build-and-release`, before `maven-central-deploy` may wait for human approval. If the tag is force-moved during that wait, this job still publishes the old output SHA even though the release tag now identifies another commit. Pinning checkout preserves consistency within the run but does not preserve the workflow's tag-as-source-of-truth invariant. Immediately after approval and before checkout or staging, resolve and peel `needs.build-and-release.outputs.tag_name` again and abort unless it still equals `needs.build-and-release.outputs.sha`.
- [SUGGESTION] .github/workflows/kotlin-sdk-release.yml:352-391: Pin actions preceding credentialed publishing steps to immutable SHAs
The protected job invokes actions through mutable major-version refs, including `android-actions/setup-android@v3` and `gradle/actions/setup-gradle@v4`, before Gradle receives the Maven Central token and GPG private key. Compromise of an action publisher or movement of one of these tags could modify the checkout, Gradle configuration, PATH, or runner state so the later credentialed commands expose those secrets. Pin every action in this credential-bearing job to a reviewed full commit SHA and use automated dependency updates to advance the pins.
| # Always an explicit refs/tags/<tag> (validated above) or the | ||
| # tag-push github.ref — never the raw dispatch input — so the | ||
| # released AAR is built from the tag's commit and a manual run can | ||
| # never build from a branch. | ||
| ref: ${{ steps.release-ref.outputs.checkout_ref }} |
There was a problem hiding this comment.
🔴 Blocking: Mutable tag checkout can substitute code behind the environment approval
The resolver validates a tag name but passes the mutable tag ref to actions/checkout. For a tag-push event created at commit A, a writer can force-move the tag to commit B before checkout; checkout then builds B, and the later tag guard also observes B, so it passes. The protected deployment consequently checks out B and executes B's Gradle wrapper with the signing and Maven credentials even though the workflow run and pending environment approval originated from the push of A. Resolve an immutable commit before checkout: use the event SHA for tag pushes, peel the selected tag to a commit SHA for manual dispatches, checkout that SHA, and retain the later tag-to-SHA guards.
source: ['codex']
There was a problem hiding this comment.
Resolved in 3e83234 — Mutable tag checkout can substitute code behind the environment approval no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - name: Checkout repository | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| # Pin to the exact SHA the build job resolved, NOT the tag ref: the | ||
| # deploy job runs after an arbitrarily long human-approval pause, and | ||
| # a tag is mutable. Checking out the SHA guarantees the staged Maven | ||
| # source is the same commit that produced the attached AAR + the | ||
| # native libs downloaded below, even if the tag was force-moved. | ||
| ref: ${{ needs.build-and-release.outputs.sha }} |
There was a problem hiding this comment.
🔴 Blocking: Revalidate the release tag after the approval delay
The only tag-to-built-SHA check runs in build-and-release, before maven-central-deploy may wait for human approval. If the tag is force-moved during that wait, this job still publishes the old output SHA even though the release tag now identifies another commit. Pinning checkout preserves consistency within the run but does not preserve the workflow's tag-as-source-of-truth invariant. Immediately after approval and before checkout or staging, resolve and peel needs.build-and-release.outputs.tag_name again and abort unless it still equals needs.build-and-release.outputs.sha.
source: ['codex']
There was a problem hiding this comment.
Resolved in 3e83234 — Revalidate the release tag after the approval delay no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - name: Checkout repository | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| # Pin to the exact SHA the build job resolved, NOT the tag ref: the | ||
| # deploy job runs after an arbitrarily long human-approval pause, and | ||
| # a tag is mutable. Checking out the SHA guarantees the staged Maven | ||
| # source is the same commit that produced the attached AAR + the | ||
| # native libs downloaded below, even if the tag was force-moved. | ||
| ref: ${{ needs.build-and-release.outputs.sha }} | ||
|
|
||
| - name: Set up JDK 17 | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: temurin | ||
| java-version: '17' | ||
|
|
||
| - name: Set up Android SDK | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: android-actions/setup-android@v3 | ||
| with: | ||
| # No Rust/cargo here — the native libraries are downloaded below. The | ||
| # NDK is still needed because AGP strips the packaged .so at assemble | ||
| # time (ndkVersion is pinned in sdk/build.gradle.kts). | ||
| packages: >- | ||
| platforms;android-35 | ||
| build-tools;35.0.0 | ||
| ndk;28.1.13356709 | ||
|
|
||
| - name: Restore native libraries from the build job | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: kotlin-sdk-jnilibs | ||
| path: packages/kotlin-sdk/sdk/src/main/jniLibs | ||
|
|
||
| - name: Setup Gradle | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| uses: gradle/actions/setup-gradle@v4 |
There was a problem hiding this comment.
🟡 Suggestion: Pin actions preceding credentialed publishing steps to immutable SHAs
The protected job invokes actions through mutable major-version refs, including android-actions/setup-android@v3 and gradle/actions/setup-gradle@v4, before Gradle receives the Maven Central token and GPG private key. Compromise of an action publisher or movement of one of these tags could modify the checkout, Gradle configuration, PATH, or runner state so the later credentialed commands expose those secrets. Pin every action in this credential-bearing job to a reviewed full commit SHA and use automated dependency updates to advance the pins.
source: ['codex']
There was a problem hiding this comment.
Resolved in 3e83234 — Pin actions preceding credentialed publishing steps to immutable SHAs no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
dashpay#4193 review) Resolve an immutable commit SHA before checkout in build-and-release (event SHA for tag pushes, peeled tag commit for manual dispatches) so a force-moved tag cannot substitute code behind the maven-central approval. Re-validate the release tag still resolves to the built/approved SHA in the gated deploy job, immediately after approval and before checkout or staging. Pin the credential-bearing deploy job's actions to full commit SHAs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@thepastaclaw addressed both in
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/kotlin-sdk-release.yml (4)
303-309: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin
softprops/action-gh-releaseto a commit SHA.This step receives the repo token and mutates the GitHub release; the
@v2major ref is mutable. The deploy job already pins every action to a full SHA — apply the same policy here for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/kotlin-sdk-release.yml around lines 303 - 309, Update the “Attach AAR to release” step’s softprops/action-gh-release reference from the mutable `@v2` tag to the approved full commit SHA, matching the SHA-pinning policy used by other actions in the deploy job. Preserve the existing release inputs and condition.
449-451: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winGradle caching in the credential-bearing job is a poisoning vector.
setup-gradleenables read/write caching by default, so this job restores cache entries written by unprotected jobs/workflows (including the build job in the same repo). A poisoned entry can alter Gradle configuration or wrapper/plugin resolution in the steps that then receive the Central token and GPG key. Make the cache read-only or disable it here.Based on zizmor's `cache-poisoning` finding at line 451.🔒 Proposed hardening
- name: Setup Gradle if: steps.secrets-gate.outputs.proceed == 'true' uses: gradle/actions/setup-gradle@48b5f213c81028ace310571dc5ec0fbbca0b2947 # v4 + with: + # Never write (or trust a writable) cache in the credentialed deploy + # job — a poisoned entry from an unprotected job could alter the + # Gradle build that receives the Central token + GPG key. + cache-read-only: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/kotlin-sdk-release.yml around lines 449 - 451, Update the credential-bearing Gradle setup step using the `gradle/actions/setup-gradle` action to prevent cache writes and untrusted cache restoration, by configuring its caching behavior as read-only or disabled. Keep the existing secrets-gate condition and Gradle setup intact.Source: Linters/SAST tools
143-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTag name is derived twice from raw event inputs.
release-refalready validated and normalized the dispatch input (existence inrefs/tags/, no slashes), but this step re-derivesTAGfromgithub.event.inputs.tag || github.ref_namewith its own prefix stripping. Emitting the normalized tag as arelease-refoutput and consuming it here removes the chance of the two derivations diverging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/kotlin-sdk-release.yml around lines 143 - 154, The sdk-version step redundantly derives and normalizes the tag instead of using the validated value from release-ref. Update the release-ref step to expose its normalized tag as an output, then consume that output in the sdk-version step via its needs dependency; remove the raw event-input fallback and local refs/tags stripping while preserving the existing tag_name output.
125-132: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBoth
actions/checkoutsteps persist theGITHUB_TOKENin.git/config. Neither job pushes to git, so the defaultpersist-credentials: trueleaves a usable token on disk for the rest of each job — including the job holding the Central token and GPG key.
.github/workflows/kotlin-sdk-release.yml#L125-L132: addpersist-credentials: falseto the build job's checkout (this job also uploads artifacts)..github/workflows/kotlin-sdk-release.yml#L409-L421: addpersist-credentials: falseto the deploy job's pinned checkout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/kotlin-sdk-release.yml around lines 125 - 132, Disable checkout credential persistence by setting persist-credentials to false on both actions/checkout steps in .github/workflows/kotlin-sdk-release.yml:125-132 and .github/workflows/kotlin-sdk-release.yml:409-421. Apply the same change to the build checkout and the deploy job’s pinned checkout; no other workflow behavior should change.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/kotlin-sdk/PUBLISHING.md`:
- Around line 25-35: Update the deployment documentation to state that
maven-central-deploy checks out the pinned SHA from build-and-release outputs,
while workflow_dispatch resolves the supplied tag to its commit SHA before
checkout. Preserve the existing explanation of version derivation and tag
handling, but do not describe either path as directly checking out the tag.
---
Nitpick comments:
In @.github/workflows/kotlin-sdk-release.yml:
- Around line 303-309: Update the “Attach AAR to release” step’s
softprops/action-gh-release reference from the mutable `@v2` tag to the approved
full commit SHA, matching the SHA-pinning policy used by other actions in the
deploy job. Preserve the existing release inputs and condition.
- Around line 449-451: Update the credential-bearing Gradle setup step using the
`gradle/actions/setup-gradle` action to prevent cache writes and untrusted cache
restoration, by configuring its caching behavior as read-only or disabled. Keep
the existing secrets-gate condition and Gradle setup intact.
- Around line 143-154: The sdk-version step redundantly derives and normalizes
the tag instead of using the validated value from release-ref. Update the
release-ref step to expose its normalized tag as an output, then consume that
output in the sdk-version step via its needs dependency; remove the raw
event-input fallback and local refs/tags stripping while preserving the existing
tag_name output.
- Around line 125-132: Disable checkout credential persistence by setting
persist-credentials to false on both actions/checkout steps in
.github/workflows/kotlin-sdk-release.yml:125-132 and
.github/workflows/kotlin-sdk-release.yml:409-421. Apply the same change to the
build checkout and the deploy job’s pinned checkout; no other workflow behavior
should change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c4d38226-e953-4eea-a4a5-599426debe12
📒 Files selected for processing (2)
.github/workflows/kotlin-sdk-release.ymlpackages/kotlin-sdk/PUBLISHING.md
| In CI the version is never passed by hand — it comes only from the tag — so | ||
| the Maven Central artifact and the GitHub-release AAR are always the same | ||
| commit under the same version (the deploy job checks out the same tag and | ||
| re-stages the native libraries the build job produced, rather than rebuilding | ||
| them). Tags that would derive a `-SNAPSHOT` version are rejected. A | ||
| `kotlin-sdk-*` tag without the `vX.Y.Z` form still gets its GitHub-release AAR | ||
| but skips the Maven deploy (there is no version to derive). The workflow's | ||
| manual `workflow_dispatch` path is for re-runs/emergencies only: it takes an | ||
| *existing* tag name — either the short `kotlin-sdk-vX.Y.Z` form or a full | ||
| `refs/tags/kotlin-sdk-vX.Y.Z` ref — checks out that tag, and derives the | ||
| version from it the same way; it has no version input. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Docs still say the deploy checks out the tag; the workflow checks out the pinned SHA.
maven-central-deploy checks out needs.build-and-release.outputs.sha, and workflow_dispatch peels the tag to a commit SHA before checkout. Describing both as "checks out that tag" hides the force-move protection that is the point of the pinning.
📝 Proposed wording
-commit under the same version (the deploy job checks out the same tag and
-re-stages the native libraries the build job produced, rather than rebuilding
-them). Tags that would derive a `-SNAPSHOT` version are rejected. A
+commit under the same version (the deploy job checks out the exact commit SHA
+the build job built — not the mutable tag ref — and re-stages the native
+libraries that build produced, rather than rebuilding them; the tag is
+re-validated against that SHA after approval). Tags that would derive a
+`-SNAPSHOT` version are rejected. A
`kotlin-sdk-*` tag without the `vX.Y.Z` form still gets its GitHub-release AAR
but skips the Maven deploy (there is no version to derive). The workflow's
manual `workflow_dispatch` path is for re-runs/emergencies only: it takes an
*existing* tag name — either the short `kotlin-sdk-vX.Y.Z` form or a full
-`refs/tags/kotlin-sdk-kotlin-sdk-vX.Y.Z` ref — checks out that tag, and derives the
-version from it the same way; it has no version input.
+`refs/tags/kotlin-sdk-vX.Y.Z` ref — resolves it to the commit it currently
+points at, checks out that commit, and derives the version from the tag the
+same way; it has no version input.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/kotlin-sdk/PUBLISHING.md` around lines 25 - 35, Update the
deployment documentation to state that maven-central-deploy checks out the
pinned SHA from build-and-release outputs, while workflow_dispatch resolves the
supplied tag to its commit SHA before checkout. Preserve the existing
explanation of version derivation and tag handling, but do not describe either
path as directly checking out the tag.
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Carried-forward prior findings: A force-moved tag can still split the GitHub and Maven artifacts remains blocking, while immutable checkout, the requested post-approval revalidation, and immutable pins in the credential-bearing job are fixed. New findings in latest delta: the new tag check still races the later irreversible deployment, and the artifact-producing build job still uses mutable actions whose output this PR now signs and publishes to Maven Central.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking | 🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/kotlin-sdk-release.yml`:
- [BLOCKING] .github/workflows/kotlin-sdk-release.yml:388-407: The tag can move again after the post-approval check
The new check runs after environment approval, but checkout, JDK and Android setup, artifact restoration, Gradle setup, and signed staging all occur before `jreleaserDeploy` at lines 475-487. A repository writer can move the tag after line 407, causing the workflow to publish the built commit under a tag that now identifies different source. Rechecking immediately before deployment narrows this interval but cannot make a mutable Git ref and an external publication atomic. Protect `kotlin-sdk-*` release tags against updates and deletion, and retain an immediate pre-deploy revalidation as defense in depth.
- [SUGGESTION] .github/workflows/kotlin-sdk-release.yml:125-255: Pin the mutable build actions whose output is signed and published
The protected deploy job is pinned, but its Maven artifact incorporates native libraries produced by `build-and-release`, where checkout, Java and Android setup, the Rust toolchain, caching, and Gradle setup still use mutable action refs. Compromise or movement of one of those refs can alter the source tree or generated `.so` files before they are uploaded; the protected job then downloads, signs, and publishes that output without independently rebuilding or attesting it. This PR newly promotes those outputs from a GitHub release asset into the Maven Central trust boundary. Pin every artifact-producing action to a reviewed immutable commit SHA and advance the pins through dependency updates.
| - name: Revalidate release tag resolves to the built commit (post-approval) | ||
| if: steps.secrets-gate.outputs.proceed == 'true' | ||
| env: | ||
| TAG: ${{ needs.build-and-release.outputs.tag_name }} | ||
| BUILT_SHA: ${{ needs.build-and-release.outputs.sha }} | ||
| REPO: ${{ github.repository }} | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| # Resolve the tag's current commit, dereferencing an annotated tag to | ||
| # the commit it points at. | ||
| OBJ_TYPE=$(gh api "repos/${REPO}/git/ref/tags/${TAG}" --jq '.object.type') | ||
| OBJ_SHA=$(gh api "repos/${REPO}/git/ref/tags/${TAG}" --jq '.object.sha') | ||
| if [ "$OBJ_TYPE" = "tag" ]; then | ||
| OBJ_SHA=$(gh api "repos/${REPO}/git/tags/${OBJ_SHA}" --jq '.object.sha') | ||
| fi | ||
| if [ "$OBJ_SHA" != "$BUILT_SHA" ]; then | ||
| echo "::error::Tag ${TAG} now points at ${OBJ_SHA} but this release was built and approved for ${BUILT_SHA} — the tag was force-moved during the approval pause. Refusing to publish so the Maven Central version cannot be released under a tag that identifies a different commit." | ||
| exit 1 | ||
| fi | ||
| echo "::notice::Tag ${TAG} still resolves to the approved commit ${BUILT_SHA} — proceeding to publish." |
There was a problem hiding this comment.
🔴 Blocking: The tag can move again after the post-approval check
The new check runs after environment approval, but checkout, JDK and Android setup, artifact restoration, Gradle setup, and signed staging all occur before jreleaserDeploy at lines 475-487. A repository writer can move the tag after line 407, causing the workflow to publish the built commit under a tag that now identifies different source. Rechecking immediately before deployment narrows this interval but cannot make a mutable Git ref and an external publication atomic. Protect kotlin-sdk-* release tags against updates and deletion, and retain an immediate pre-deploy revalidation as defense in depth.
source: ['codex']
Stacked on #4182 (Maven publishing) — merge after it. The branch includes #4182's commits until it lands; the last commits are this PR's content.
Follow-up to #4182, addressing @shumkov's review point: nothing tied
-PsdkVersionon the manual Maven publish to thekotlin-sdk-*tag that drives the GitHub-release AAR, so the two channels could ship different commits under one version.This folds
jreleaserDeployintokotlin-sdk-release.yml:kotlin-sdk-vX.Y.Z→-PsdkVersion=X.Y.Z; the checkout is already pinned to the same tag, so the Maven Central artifact and the GitHub-release AAR are always the same commit under the same version. There is no version input anywhere in the workflow. Malformedkotlin-sdk-v*tags and tag-derived-SNAPSHOTversions fail the run (early, before the native build); non-versionedkotlin-sdk-*tags keep today's AAR-only behavior.org.dashjis currently personal, so the deploy steps skip with a notice when the secrets (JRELEASER_MAVENCENTRAL_SONATYPE_*,JRELEASER_GPG_*) aren't configured — the tag still produces the GitHub-release AAR, and the documented manual runbook (now explicitly "from the tag's commit with the tag's version") remains the fallback. A partially configured secret set fails hard instead of guessing.cleanStagingDeploy,verifyJniLibsForRemotePublishandverifyStagedAarForRemotePublishare wired via task dependencies and appear in the--dry-rungraph for both the staging and deploy invocations used by the workflow.workflow_dispatchstays, tag-derived. It only accepts an existing tag name (re-run/emergency path); the version is derived from that tag exactly as on tag push.PUBLISHING.mddocuments the CI path as preferred and demotes the manual runbook to a fallback.Verified: workflow parses (
yaml.safe_load); tag→version derivation exercised against valid/invalid/snapshot/injection-shaped tags; both Gradle invocations dry-run with the guards in the task graph (JDK 17).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation