Merge main into the Valdi Web sync branch (resolve #148 conflicts) - #188
beaucollins wants to merge 62 commits into
Conversation
GitOrigin-RevId: 5f03147b5f519830b87930863bc6299369f212c1
GitOrigin-RevId: fecd854ba919a4bf0b11e2af277b361bac7bc8c3
GitOrigin-RevId: 11e652f9525ebb9c54c9e7615ae3db5306064426
GitOrigin-RevId: 2b9143143a39f4fb5565eb60ab7fe6322b8ad958
GitOrigin-RevId: 988234cc6bc89ec9caf1d9e03a74bb637e6ff0ec
GitOrigin-RevId: 9dffe495ea56dc760f9d1406670ceefbeae5b959
GitOrigin-RevId: 1518ae4c6b0842716245ac5d368d3785b0241ad9
GitOrigin-RevId: eb076955e5df1bfe02fcf7a44060975c4973da70
GitOrigin-RevId: c3638d2626349469532154d69aec8f6c7feb931b
GitOrigin-RevId: 3f52e8d9b3fe4b7f8b2cfffbf1d064f7211e3252
GitOrigin-RevId: 26e7ed79d645645fe2e051079e1c7c149787685d
GitOrigin-RevId: a37913a541b813cf6c2c8301d2659dcee412630a
GitOrigin-RevId: c6accff718b92cea3345849fa628ee372cab5d0f
GitOrigin-RevId: 90787ce091e5a3258df673d2e740597bf1ebb9cf
GitOrigin-RevId: d2bdc301e68f09d33d9706221cb53cae96e28da6
GitOrigin-RevId: be39f31edaed08d8a9d76746895a7f0779b58645
GitOrigin-RevId: e541a245cd6170bf357ec481c184b3b49d444921
GitOrigin-RevId: 0166f2d10b88f50cae91367da53badf94ebedf36
GitOrigin-RevId: 12f800810e37f49d8f69dc0e19e415e22d28b2b7
GitOrigin-RevId: 62594644db995fc9f1eff2321c1e47ac1cae8345
GitOrigin-RevId: 1597338e8a80d81b952f0f6de706d639574e7fe6
GitOrigin-RevId: 21fdc2d8a35a725400517c61a7fa2b770761f032
GitOrigin-RevId: 2cb0a6bd7a78f308af413137ef8e085aa81e8de0
GitOrigin-RevId: 07ec6e8a7a8097d40ea19aed9812be6afa5b14d0
GitOrigin-RevId: 07a32d08604745982a9c0d58ec8ca1579da300df
GitOrigin-RevId: ad15a79cb318dcfc9532f351abfe855b2329f341
GitOrigin-RevId: d84ee7ebec019ecba348a3cc92fccf61f32f6e84
GitOrigin-RevId: c6f3b724dd5ec4bbbb6720bc6c894ddd692f769a
GitOrigin-RevId: 18dac7571060b7e860dbae641ea9f7e809c39cc7
GitOrigin-RevId: 6e179754513f80c6b79d31d8570d52ea05bfdce7
GitOrigin-RevId: 5150e1441fbaf206af3b6da0e3995bbe685ad141
GitOrigin-RevId: 7517d98b032b4d4fd2467e19c6da5e864e428ce9
GitOrigin-RevId: 8bd1a0f99c2b1b468c275a2ac91af98839c73eb8
GitOrigin-RevId: c07ff7093330598c76aa5cd49762b9522a7218a4
GitOrigin-RevId: eebe4496de3bf99b180582ebd0eebad517bc3160
GitOrigin-RevId: 4fe81f39227b9deb28693175fd0da59649da24a1
GitOrigin-RevId: 5e0e3ce5bfb064677167bc6683a04d3cf81f348d
GitOrigin-RevId: 82786be6991361a40dd3d4746e52c690a7331f91
GitOrigin-RevId: 4f1751471ea604d53f9562e3028b917301d702e0
GitOrigin-RevId: 425d0864a1db275dc8c2f78c3124217e0c69212b
GitOrigin-RevId: 9673d5c6ef3f3cd2492c4af9c0a987679e5b979d
GitOrigin-RevId: 8cddb3fb30b730351beaafc44997dfd9e1af779c
main and simon/260814-valdi-web have unrelated histories (no common ancestor), so every file present in both trees with differing content surfaced as an add/add conflict — 313 in total. These are an artifact of the missing merge base, not competing edits. Resolution: for all 313 conflicts, take the sync branch's version. The sync branch is the authoritative 731-commit import that the web build and tests were validated against; main is a 14-commit skeleton whose only divergences are stale dependency pins and superset-able infra files (.bazelrc, CI workflows, MODULE.bazel). Taking the sync side keeps the tree internally consistent with the code it was built and tested against. The 446 files that exist only on main are carried in unchanged by the merge (they already live on main, so this is a no-op there). Flagged for author review in case the sync intends to supersede any of them.
Sensitive Files Detected🔧 Build rules — Affects build rules for all Valdi consumers. 🧬 Compiler emit goldens regenerated — The Valdi compiler produced different output for the testdata corpus (e.g. a TypeScript bump or a transform change). Review the emit diff carefully and confirm it is intended — see tools/ci/compiler_goldens/README.md. This is an automated notice. A maintainer will review after import. |
📊 PR Size: size/XLTotal changes: 65035 lines (446 files) Top files changed:
...and 436 more files Size calculated as additions + deletions. Labels: XS (<10), S (<50), M (<250), L (<1000), XL (1000+) |
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Java 17 | ||
| uses: actions/setup-java@v4 |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
actions/setup-java@v4 is a mutable action reference executed with cloud-related workflow permissions. If its tag is repointed, attacker-controlled code could run before the build and steal credentials or poison the Bazel cache.
More details about this
actions/setup-java@v4 uses the mutable v4 tag rather than an immutable commit reference. The action runs before Configure Bazel cache, setup_linux_env.sh, and sanitizer_tests.sh, so if the v4 tag is repointed or the action repository is compromised, the workflow could execute attacker-controlled Java setup code on the ubuntu-latest runner. That code could read available workflow variables and credentials, alter the checkout or build environment used by the Valdi ASan tests, and—because this workflow grants id-token: write—attempt to obtain an OIDC token and access any cloud role trusted by GCP_PROJECT_ID, WORKLOAD_IDENTITY_PROVIDER, or GCP_SERVICE_ACCOUNT.
A plausible attack is: (1) an attacker causes the v4 tag in actions/setup-java to point to a malicious commit; (2) the scheduled or manually dispatched valdi-sanitizer-tests workflow executes that commit during Setup Java 17; (3) the malicious action inspects the runner environment and workflow context, then requests an OIDC token or uses accessible credentials; (4) the attacker uses the obtained cloud access to read or modify the Bazel cache bucket associated with BAZEL_CACHE_BUCKET, potentially poisoning artifacts consumed by later builds. Because the tag can move without any change to this workflow, the workflow's reviewed dependency is not stable.
To resolve this comment:
✨ Commit fix suggestion
- Replace the mutable
actions/setup-java@v4reference with the full 40-character commit SHA for the trustedactions/setup-javarelease, for example:uses: actions/setup-java@<40-character-commit-sha> # v4. - Verify that the commit SHA belongs to the official
actions/setup-javarepository and corresponds to the intended v4 release before applying it. - Keep the existing
distributionandjava-versioninputs unchanged.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by github-actions-mutable-action-tag.
You can view more details about this finding in the Semgrep AppSec Platform.
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
actions/checkout@v4 can silently change to attacker-controlled code. A compromised tag could execute on scheduled or manually dispatched runs with repository-read and OIDC permissions, exposing source code or trusted cloud access.
More details about this
actions/checkout@v4 uses the mutable v4 tag rather than an immutable commit. The action runs at the start of valdi-sanitizer-tests on a GitHub-hosted runner, and this workflow grants contents: read plus id-token: write; if the action owner or its release process repoints v4, a later scheduled run or manual dispatch could execute attacker-controlled action code before the repository steps run.
A plausible attack would be:
- An attacker compromises the
actions/checkoutrelease path or obtains control of thev4tag. - The next
schedulerun resolvesactions/checkout@v4to the attacker's commit instead of the expected checkout implementation. - That code executes with this job's permissions, reads the checked-out repository and available workflow context, and can request an OIDC token because
id-token: writeis enabled. - The attacker sends repository contents, the
GITHUB_TOKEN, or a cloud credential exchanged from the OIDC token to an external server, potentially allowing source-code theft or access to trusted cloud resources.
The same mutable-reference risk also exists for actions/setup-java@v4 later in this workflow, although this finding is on the checkout step.
To resolve this comment:
✨ Commit fix suggestion
-
Replace the mutable checkout tag with the full commit SHA for the trusted
actions/checkoutv4 release:
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 -
Pin the external
actions/setup-java@v4step to the full 40-character commit SHA for the approved v4 release, retaining a version comment such as# v4. -
Leave the local action reference
uses: ./.github/actions/bazel-cacheunchanged; local paths are not mutable external dependencies.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by github-actions-mutable-action-tag.
You can view more details about this finding in the Semgrep AppSec Platform.
|
Semgrep found 60
This literal might contain a Snapchat internal reference that should not be committed to open-source repositories. Fix: Please replace / remove the string to avoid committing it to open-source repositories. |
What
Merges
maininto the Valdi Web sync branch (simon/260814-valdi-web), resolving the conflicts that block #148 from merging tomain.Why the conflicts exist
mainandsimon/260814-valdi-webhave unrelated histories — zero shared commits, no merge base. So every file present in both trees with differing content surfaced as anadd/addconflict (313 of them). They are an artifact of the missing common ancestor, not competing edits. Of the 313, 312 are real content differences (205 edited on both sides, 79 trimmed on head, 29 head-superset); only 1 is whitespace-only.How it's resolved
All 313 conflicts take the sync branch's version — it's the authoritative 731-commit import the web build and tests were validated against, whereas
mainis a 14-commit snapshot whose divergences are stale dependency pins and superset-able infra. The resolved tree is byte-identical to the sync head for those files.The 446 files that exist only on
mainare carried in unchanged by the merge (they already live onmain).Effect
After this lands on
simon/260814-valdi-web,mainbecomes an ancestor of the sync branch, so #148 merges tomainconflict-free.For reviewers
Taking the sync side drops a few main-only changes to shared files — notably
main's ASan nightly-sanitizer block in.bazelrc. Please confirm those are intentionally superseded, and review the 446 main-only files for any that the sync means to replace.🤖 Generated with Claude Code