Skip to content

[Merged by Bors] - ci: Secure proofwidgets fetches on cache get#35463

Closed
marcelolynch wants to merge 3 commits intomasterfrom
ci-dev/marcelo/2026/02/SkipProofWidgets
Closed

[Merged by Bors] - ci: Secure proofwidgets fetches on cache get#35463
marcelolynch wants to merge 3 commits intomasterfrom
ci-dev/marcelo/2026/02/SkipProofWidgets

Conversation

@marcelolynch
Copy link
Contributor

When calling lake exe cache get in the CI we happily say "only runs cache get from tools-branch, so doesn't need to be inside landrun".

However, it turns out that cache get is not that innocent: it performs a full lake -v build proofwidgets:release in the PR branch context. This means I can point proofwidgets in the lake-manifest.json to whatever I want, and now run arbitrary code outside of landrun.

To protect against this, this PR:

  • adds an dependency verification for proofwidget, so that it only comes from the 'trusted' source
  • tries to avoid the build altogether in cache get, defaulting to skipping in github actions, but adding a flag so that we can do it if we want

@github-actions
Copy link

PR summary ac6e242131

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ parseFlagOpt

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for scripts/declarations_diff.sh contains some details about this script.


No changes to technical debt.

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

Workflow documentation reminder

This PR modifies files under .github/workflows/.
Please update docs/workflows.md if the workflow inventory, triggers, or behavior changed.

Modified workflow files:

  • .github/workflows/build_template.yml

@github-actions github-actions bot added the CI Modifies the continuous integration setup or other automation label Feb 17, 2026
@marcelolynch
Copy link
Contributor Author

@marcelolynch marcelolynch requested a review from kim-em February 17, 2026 23:12
@joneugster joneugster self-assigned this Feb 18, 2026
Copy link
Contributor

@joneugster joneugster left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

maintainer merge

@github-actions
Copy link

🚀 Pull request has been placed on the maintainer queue by joneugster.

@mathlib-triage mathlib-triage bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Feb 18, 2026
@bryangingechen
Copy link
Contributor

Thanks!
bors r+

@mathlib-triage mathlib-triage bot added ready-to-merge This PR has been sent to bors. and removed maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. labels Feb 18, 2026
mathlib-bors bot pushed a commit that referenced this pull request Feb 18, 2026
When calling lake exe cache get in the CI we happily say ["only runs cache get from tools-branch, so doesn't need to be inside landrun"](https://github.com/leanprover-community/mathlib4/blob/a90320db82953c5554b9065b7304c10e3b4548a5/.github/workflows/build_template.yml#L293).

However, it turns out that cache get is not that innocent: it [performs a full lake -v build proofwidgets:release](https://github.com/leanprover-community/mathlib4/blob/a90320db82953c5554b9065b7304c10e3b4548a5/Cache/Requests.lean#L451) in the PR branch context. This means I can point proofwidgets in the lake-manifest.json to whatever I want, and now run arbitrary code outside of landrun.

To protect against this, this PR:
- adds an dependency verification for proofwidget, so that it only comes from the 'trusted' source
- tries to avoid the build altogether in `cache get`, defaulting to skipping in github actions, but adding a flag so that we can do it if we want
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 18, 2026

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title ci: Secure proofwidgets fetches on cache get [Merged by Bors] - ci: Secure proofwidgets fetches on cache get Feb 18, 2026
@mathlib-bors mathlib-bors bot closed this Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Modifies the continuous integration setup or other automation ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments