Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,25 @@ jobs:
release-please:
if: github.repository == 'sentdm/sent-dm-java'
runs-on: ubuntu-latest
env:
HAS_APP: ${{ secrets.SDK_APP_ID != '' && secrets.SDK_APP_PRIVATE_KEY != '' }}

steps:
- name: Mint the release token from the GitHub App
# A tag or release created with the built-in token starts no
Comment on lines 17 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Anyone reading this public repo's commit history or PR can see a private Sent repository and an internal ticket, which CLAUDE.md forbids. Commit 8b2e6de names the "sent-dm-sdk-pipeline App" and the "seal dispatch". The PR text also cites "EPIC-4427". sent-dm-sdk-pipeline is Sent's private config repo (seal-dispatch.yml:37 CONFIG_REPO: sentdm/sent-dm-sdk-pipeline). Fix: reword the commit message and PR description so they say only "the GitHub App", with no internal repo name and no ticket ID, before merging.

Why this was flagged

CLAUDE.md says: "This repository is public. Nothing in it, including PR text, CHANGELOG.md and release notes, links to or names Sent's private repositories, internal systems or providers." REVIEW.md also lists naming a private Sent repository or an internal system as Important. The message of commit 8b2e6de says "The token now comes from the sent-dm-sdk-pipeline App, as the seal dispatch's already does". The PR description names sent-dm-sdk-pipeline and "EPIC-4427", an internal tracker ID. seal-dispatch.yml:37 shows that sentdm/sent-dm-sdk-pipeline is the private config repo. Once merged, the commit message stays permanently in main's public history. The ci: type keeps it out of the changelog, but the history itself is still public.

Verification: The PR's only commit, 8b2e6de, says in its body: "The token now comes from the sent-dm-sdk-pipeline App, as the seal dispatch's already does". In the base, .github/workflows/seal-dispatch.yml:37 has CONFIG_REPO: sentdm/sent-dm-sdk-pipeline. The PR description also cites "EPIC-4427", an internal tracker ID.

# workflow, so the publish workflow would never run; a personal
# access token expires or dies with its owner's account. The App's
# token does neither.
id: app
if: env.HAS_APP == 'true'
uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v3.3.0
with:
app-id: ${{ secrets.SDK_APP_ID }}
private-key: ${{ secrets.SDK_APP_PRIVATE_KEY }}
owner: sentdm
repositories: ${{ github.event.repository.name }}

Comment on lines +20 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) If the App is not installed on this repository, or lacks write grants here, releases stay broken and the PAT fallback never kicks in. HAS_APP checks only that the secrets exist, not that a usable token can be minted. The mint step has no permission-* inputs and no continue-on-error, so a missing installation fails the job at mint. A missing grant fails it later inside release-please. Fix: confirm the App installation covers sent-dm-java, and request permission-contents: write and permission-pull-requests: write explicitly so a missing grant fails at mint with a clear error. The only other installation in this checkout is repositories: sent-dm-sdk-pipeline in seal-dispatch.yml.

Why this was flagged

The trigger is a configuration state this checkout cannot show. Either the App installation does not include sentdm/sent-dm-java, or it lacks contents and pull-requests write there. The only existing use, .github/workflows/seal-dispatch.yml:47, mints for repositories: sent-dm-sdk-pipeline only, so nothing here shows access to this repo. Once the secrets exist, HAS_APP at release-please.yml:16 evaluates true, and the step at lines 25-32 runs with repositories: ${{ github.event.repository.name }}. With no installation on this repo, create-github-app-token fails and the job stops, because the step has no continue-on-error. If the installation exists but lacks write grants, the unscoped token reaches release-please at line 36, and tagging fails with 403. In both cases the || secrets.RELEASE_PLEASE_TOKEN fallback never runs, and that PAT is revoked anyway. So the merged release PR still cuts no tag and publish-sonatype.yml never runs, same as on the base branch, even though the PR says merging cuts the release. Nothing in the workflow checks that the minted token can write to this repository.

Verification: nit. Trigger: the SDK_APP_ID/SDK_APP_PRIVATE_KEY secrets are visible to this repo, but the App installation does not include sentdm/sent-dm-java. The mint step (lines 20-32) has no continue-on-error. If it can't mint a token for this repo, the job stops there, so steps.app.outputs.token || secrets.RELEASE_PLEASE_TOKEN (line 36) never gets to use the PAT.

- uses: googleapis/release-please-action@5c625bfb5d1ff62eadeeb3772007f7f66fdcf071 # v4.4.1
id: release
with:
token: ${{ secrets.RELEASE_PLEASE_TOKEN }}
token: ${{ steps.app.outputs.token || secrets.RELEASE_PLEASE_TOKEN }}
Comment on lines +15 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Maintainers can lose this fix, or block every SDK's release, when the next regeneration rewrites this release workflow. CLAUDE.md lists "the publish, release and sync workflows" as generator-owned, and commit 8b2e6de has no Stainless-Generated-From trailer. It also changes the generator-written token: line (line 36), not only adding lines, so the patch conflicts on regeneration. Fix: make the App-token change in the SDK configuration so a generated commit carries it. If a local patch must stay, keep it to added lines and leave the generator's token: line untouched.

Why this was flagged

CLAUDE.md says "Generator-owned, so don't edit them here: ... the publish, release and sync workflows" and "Changing lines the generator writes conflicts the next time the API document changes, and a conflict blocks every SDK's release". REVIEW.md asks for any change to a generator-owned file outside a generated commit to be reported as Important. The only commit in range, 8b2e6de "ci(repo): mint the release token from the GitHub App", has no Stainless-Generated-From trailer. Besides adding the env block and the App step (lines 15-31), it rewrites the generator's line token: ${{ secrets.RELEASE_PLEASE_TOKEN }} to token: ${{ steps.app.outputs.token || secrets.RELEASE_PLEASE_TOKEN }} at release-please.yml:36. On the next generated commit, either the generator overwrites the file and drops the App token, so Release Please goes back to the revoked PAT and fails with "Bad credentials", or the carried patch conflicts on that line and blocks the pipeline. The base branch has no hand patch on this file to conflict.

Verification: CLAUDE.md lines 19-22 say the release workflow is generator-owned. Commit 8b2e6de has no Stainless-Generated-From trailer. Line 36 of .github/workflows/release-please.yml changes the generator-written token: line. So on regeneration, the generator either rewrites the file and drops the App-token fix, or the carried patch conflicts and blocks releases.

Loading