Skip to content

Readjust the trampoline CI check#19225

Draft
konstin wants to merge 1 commit intomainfrom
konsti/rs-change-trampoline-check
Draft

Readjust the trampoline CI check#19225
konstin wants to merge 1 commit intomainfrom
konsti/rs-change-trampoline-check

Conversation

@konstin
Copy link
Copy Markdown
Member

@konstin konstin commented Apr 30, 2026

Using the docker container, we can reproducibly build the committed windows trampolines given that neither the sources nor the crate versions change, as the crate versions are part of cargo's Strict Version Hash which determines the ordering in binaries. If we mandated the committed binaries to always be up to date, every version bump would require rebuilding them. Instead, we only require updating them when any rust sources of the trampoline crate change, or the "build:windows-trampoline" is set. This allows the trampolines to go "stale", while still ensuring that when the checked-in binaries are always built from checked-in sources (security) and that updating the trampoline sources comes with an update to the binaries.

Using the docker container, we can reproducibly build the committed windows trampolines given that neither the sources nor the crate versions change, as the crate versions are part of cargo's Strict Version Hash which determines the ordering in binaries. If we mandated the committed binaries to always be up to date, every version bump would require rebuilding them. Instead, we only require updating them when any rust sources of the trampoline crate change, or the "build:windows-trampoline" is set.  This allows the trampolines to go "stale", while still ensuring that when the checked-in binaries are always built from checked-in sources (security) and that updating the trampoline sources comes with an update to the binaries.
@konstin konstin added internal A refactor or improvement that is not user-facing build:windows-trampoline labels Apr 30, 2026
@zanieb
Copy link
Copy Markdown
Member

zanieb commented Apr 30, 2026

I'm a -1 on this change, I worry it's hard to maintain and it seems plausible a malicious change could sneak through. I'd prefer we rewrote the crate versions to make the build deterministic.

@konstin
Copy link
Copy Markdown
Member Author

konstin commented Apr 30, 2026

How would a malicious change sneak in? The check runs every time the binaries are changed, it's not different from a change that rewrites the versions in that sense.

Rewriting the versions would require having a similar setup for building the binaries locally, which seems unreasonably complex (Do we modify your local checkout and modify it back in the end, assuming that we don't overwrite anything in the revert path? Do we use cargo package and then rewrite in a temp dir?).

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Apr 30, 2026

How would a malicious change sneak in? The check runs every time the binaries are changed, it's not different from a change that rewrites the versions in that sense.

Ah that's fine then. You said...

Instead, we only require updating them when any rust sources of the trampoline crate change

which confused me, as I thought that we were only performing the check based on source changes.

Rewriting the versions would require having a similar setup for building the binaries locally, which seems unreasonably complex

We're already putting everything in a container. We can just copy the tree in and mutate it? I already have code that does this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build:windows-trampoline internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants