You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ci: a publish_snapshot.yml dry run still passes the Nexus secrets to the build #7319
publish_snapshot.yml has a dry_run input that builds and verifies the jars without deploying them. A dry run skips the "Configure Maven server credentials" step and uses the install goal, but the Build and install step still sets both Nexus secrets in its environment:
So a dry run is not credential-free. Maven, its plugins and the build scripts they run can all read NEXUS_USER and NEXUS_PW, though a dry run never needs them.
This is hardening, not a known leak. The secrets are repository Actions secrets, and anyone with write access can already run a modified workflow that uses them. The point is that a run advertised as safe to try, and one that changes allows in a fork (if: github.repository == 'apache/datafusion-comet' || inputs.dry_run), should not carry the publishing credentials at all.
Proposal
Make a dry run credential-free. The smallest change is to leave the variables empty when dry_run is set:
A stronger form builds and verifies the jars in one job with no secrets, uploads them, and deploys from a second job that has the secrets and runs only deploy on the verified output. That also keeps the credentials away from the five-variant clean and compile loop in a real run.
Raised in review of #7311, which does not touch this file.
P2 — Proposed NEXUS_USER / NEXUS_PW expressions — Do not fall back to the secret during a dry run
inputs.dry_run && '' || secrets.NEXUS_USER returns the configured secret for both input values. When dry_run is true, the && result is the falsy empty string, so || selects the secret. A fork without configured secrets would conceal this error. The password expression has the same problem.
GitHub documents empty-string falsiness in its expression reference. This is an expression error in the proposed repair, not evidence of an observed credential leak.
Action: Remove this fallback pattern and verify dry-run behavior with nonempty dummy credentials. Neither publishing credential should reach a dry-run build.
P2 — Proposal scope — Require a credential-free build and publication outside its lifecycle
The original review requested both credential-free dry runs and separation of real builds from publishing credentials. Calling the separate jobs a stronger alternative allows a dry-run-only fix while Maven and its build plugins still receive the publisher credentials during real runs.
The proposed second job must not invoke the project's mvn deploy lifecycle. Maven runs the preceding lifecycle phases too, so moving that command would execute build plugins with the credentials again. The current dry-run upload also contains only binary JARs, whereas publication includes the POM and source attachments.
Evidence
A stronger form builds and verifies the jars in one job with no secrets
Action: Make the job split required. Build and verify without publishing credentials in every mode. Have the fresh restricted publisher upload the checked binary, flattened POM and existing source attachments for all five current variants, without rerunning the Comet lifecycle or importing executable build state. Skip that publisher for dry runs. Validate both routing modes with dummy credentials and verify the complete artifact set and unchanged binary checksums against a local repository, without Nexus access.
Activity
ErikBPF commentedon Oct 29, 2026
P2 — Proposed
NEXUS_USER/NEXUS_PWexpressions — Do not fall back to the secret during a dry runinputs.dry_run && '' || secrets.NEXUS_USERreturns the configured secret for both input values. Whendry_runis true, the&&result is the falsy empty string, so||selects the secret. A fork without configured secrets would conceal this error. The password expression has the same problem.Evidence
GitHub documents empty-string falsiness in its expression reference. This is an expression error in the proposed repair, not evidence of an observed credential leak.
Action: Remove this fallback pattern and verify dry-run behavior with nonempty dummy credentials. Neither publishing credential should reach a dry-run build.
P2 — Proposal scope — Require a credential-free build and publication outside its lifecycle
The original review requested both credential-free dry runs and separation of real builds from publishing credentials. Calling the separate jobs a stronger alternative allows a dry-run-only fix while Maven and its build plugins still receive the publisher credentials during real runs.
The proposed second job must not invoke the project's
mvn deploylifecycle. Maven runs the preceding lifecycle phases too, so moving that command would execute build plugins with the credentials again. The current dry-run upload also contains only binary JARs, whereas publication includes the POM and source attachments.Evidence
Action: Make the job split required. Build and verify without publishing credentials in every mode. Have the fresh restricted publisher upload the checked binary, flattened POM and existing source attachments for all five current variants, without rerunning the Comet lifecycle or importing executable build state. Skip that publisher for dry runs. Validate both routing modes with dummy credentials and verify the complete artifact set and unchanged binary checksums against a local repository, without Nexus access.