Skip to content

ci: fix cache issue#51

Open
nbelenkov wants to merge 2 commits into
solana-program:mainfrom
nbelenkov:fix_cache_issue
Open

ci: fix cache issue#51
nbelenkov wants to merge 2 commits into
solana-program:mainfrom
nbelenkov:fix_cache_issue

Conversation

@nbelenkov

Copy link
Copy Markdown
Contributor

Two hardening changes to the Rust and JS publish workflows, following pinocchio#434.

Drop caching in the publish flow

The gated publish jobs restored caches that unprotected jobs in the same flow also wrote to. Keys aren't scoped by environment and restore-keys match by prefix, so an unprotected job could seed a cache the privileged publish job later restores and runs under the publish token.

Added a cache input to setup-ubuntu (defaults to true, so CI is unchanged) that gates the pnpm, Solana, and cargo caches, and set cache: false on the publish setup steps.

Pass inputs via env vars

Free-form inputs (version, level, tag, etc.) were interpolated straight into run: scripts, allowing shell injection. They're now bound to step env: and used as quoted shell variables.

Signed-off-by: Nikita Belenkov <nikita.belenkov@anza.xyz>
@nbelenkov

Copy link
Copy Markdown
Contributor Author

cc @joncinque @yihau

@joncinque joncinque 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.

Looks great!

Comment on lines +170 to +172
env:
PACKAGE_PATH: ${{ inputs.package-path }}
run: cargo semver-checks --manifest-path "${PACKAGE_PATH}/Cargo.toml"

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.

For my own information, what exactly does the env variable provide in a case like this? The variable is already quoted, so it should end up exactly the same, no?

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