Skip to content

QVAC-18047: prevent artifact poisoning in integration + publish workflows#2317

Merged
GSServita merged 2 commits into
mainfrom
fix/critical-security-alerts-artifact-poisoning
Jun 1, 2026
Merged

QVAC-18047: prevent artifact poisoning in integration + publish workflows#2317
GSServita merged 2 commits into
mainfrom
fix/critical-security-alerts-artifact-poisoning

Conversation

@GSServita

Copy link
Copy Markdown
Contributor

Summary

Re-land the artifact-poisoning hardening that was reverted in #1871 (revert of #1728), using the win32-safe split-step pattern already proven on the VLA addon in 779ee92.

Closes the 8 open critical actions/artifact-poisoning/critical CodeQL alerts across 7 workflow files. The remaining 7 open critical actions/untrusted-checkout/critical alerts (in .github/actions/run-lint-and-unit-tests/action.yaml and .github/workflows/on-pr-ocr-onnx.yml) are deferred to a follow-up PR — their fix requires architectural changes to a shared composite action consumed by 7+ on-pr workflows and is intentionally out of scope here to keep this PR low-risk.

What changed

For each affected actions/download-artifact consumer, the artifact is now downloaded into ${{ runner.temp }}/<staging>-staging instead of straight into the workspace, then moved into place by two mutually-exclusive steps gated on matrix.platform:

  • Unix (matrix.platform != 'win32'): bash + cp -r staging/* dst/, followed by ls -la dst/ for diagnostics.
  • Windows (matrix.platform == 'win32'): powershell + Copy-Item -Recurse -Force with native Windows paths, followed by Get-ChildItem -Recurse $dst.

This is the same pattern shipped in integration-test-vla.yml lines 155-183 and proven on win32 since 779ee92.

Files touched (8 alerts, 7 files)

File Closed alerts Notes
.github/workflows/integration-test-classification-ggml.yml #849 Preserves pattern: classification-ggml-${{ matrix.platform }}-${{ matrix.arch }}* filter
.github/workflows/integration-test-llm-llamacpp.yml #848
.github/workflows/integration-test-ocr-onnx.yml #784 Uses ${{ inputs.workdir || env.PKG_DIR }} to match existing path resolution
.github/workflows/integration-test-transcription-whispercpp.yml #831
.github/workflows/integration-test-translation-nmtcpp.yml #832
.github/workflows/integration-test-tts-onnx.yml #833, #834 Both run-integration-tests (L187) and run-supertonic-desktop-benchmarks (L414) jobs
.github/workflows/publish-sdk.yml #757, #758 Ubuntu-only — single bash move step, no Windows branch

Why the previous attempt regressed

PR #1728 introduced the staging pattern but used a single bash cp -r ... 2>/dev/null \|\| true step for all platforms. On Windows runners ${{ runner.temp }} substitutes to C:\a\_temp; Git Bash interprets \a and \_ as escape sequences, the source path becomes invalid, cp -r returns 1, and the trailing 2>/dev/null \|\| true silently swallows the error. The destination ends up empty and bare's addon resolver throws the misleading ADDON_NOT_FOUND — exactly what triggered #1871. Full root-cause writeup in 779ee92.

Invariants vs #1728

  • Drop 2>/dev/null \|\| true on the Unix branch — surface real errors instead of masking them.
  • Split the move into Unix/Windows steps; Windows uses PowerShell with backslash-form paths.
  • Preserve every existing pattern: / name: / merge-multiple: / continue-on-error: attribute on the download step.
  • No change to action SHAs, permissions: blocks, or the env-var indirection for npm-token / pat-token in run-lint-and-unit-tests/action.yaml (those were already settled by Revert "fix: prevent code injection and untrusted checkout in CI workflows (#1728)" #1871 and remain on main).
  • No SDK / addon source files modified.

Test plan

Out of scope (follow-up PRs)

  • 7 actions/untrusted-checkout/critical alerts in .github/actions/run-lint-and-unit-tests/action.yaml (lines 63, 67, 71, 75) and .github/workflows/on-pr-ocr-onnx.yml (lines 131, 148).

Refs

@GSServita GSServita added the verified Authorize secrets / label-gate in PR workflows label May 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/classification-ggml (Android)

Result: passed

metric value
Devices passed 3
Devices failed 0
Test cases total 9
Test cases passed 9
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/classification-ggml (iOS)

Result: passed

metric value
Devices passed 2
Devices failed 0
Test cases total 6
Test cases passed 6
Test cases failed 0
Test cases skipped 0

View workflow run

@GSServita

Copy link
Copy Markdown
Contributor Author

CI triage — first full verified pass

I applied verified after pushing the initial commit. CI rolled all 6 affected on-pr-* workflows + the publish flow. The artifact-poisoning fix is green on the legs that matter (windows-2025 integration), and the remaining failures are unrelated infra/test issues.

✅ Primary signal — the win32 regression that reverted #1728 does NOT recur

Every Windows integration leg that finished passed:

Addon Windows integration job Result
OCR run-integration-tests / test-win32-x64 ✅ pass (13m15s)
LLM run-integration-tests / test-win32-x64 ✅ pass (12m57s)
Classification-ggml run-integration-tests / win32-x64-integration-tests ✅ pass (1m37s)
NMTCPP run-integration-tests / win32-x64-integration-tests ✅ pass (6m30s)
Whispercpp blocked before integration ⏳ darwin-x64 prebuild flaked (see below)
TTS-ONNX blocked before integration ⏳ sanity-checks pre-existing yamlfmt (see below)

CodeQL is also green — the 8 actions/artifact-poisoning/critical alerts referenced in QVAC-18612 are closed.

Unrelated failures (none caused by this PR)

  1. merge-guard / validate-pr × 6 stale — these are runs that fired before verified was applied. They explicitly say "This PR needs the 'verified' label to be merged". Newer merge-guard runs will supersede them.
  2. sanity-checks (ONNX) — Verify that yaml files are formatted step — yamlfmt flags pre-existing trailing-whitespace in files this PR does not touch (.github/actions/cpp-lint/action.yaml, detect-version-bump/action.yml, integration-test-ocr-ggml.yml, integration-test-tts-ggml.yml, integration-test-vla.yml, …) plus a few blank lines in integration-test-{transcription-whispercpp,translation-nmtcpp,tts-onnx}.yml that are also unrelated to this diff. I deliberately did NOT include a repo-wide yamlfmt sweep here; it belongs in its own PR.
  3. prebuild / prebuild / darwin-x64 (Whispercpp), cpp-tests-coverage / linux-x64-cpp-tests (Whispercpp), cpp-tests / cpp-tests (NMTCPP) — all three are the same vcpkg cache race on self-hosted runners:
    fatal: Unable to create '…/vcpkg/downloads/git-tmp/.git/shallow.lock': File exists.
    Another git process seems to be running in this repository
    
    Worth a gh run rerun --failed to clear, then Whispercpp's win32-x64 integration leg will run too.
  4. run-integration-tests / test-darwin-arm64 (LLM) — real LLM test failure ([TextLlm] failed to decode next token in finetuning pause/resume + session-cache tests). Pre-existing/flaky on darwin-arm64, unrelated to artifact staging.

Suggested reviewer actions

  • Decide whether to gh run rerun --failed on the three vcpkg-race jobs to unblock Whispercpp's Windows integration leg as additional confidence (already confirmed on 4 other addons).
  • Pre-existing yamlfmt issues and the LLM darwin-arm64 finetuning failure are out of scope for this PR.

Scope reminder

This PR only addresses the 8 actions/artifact-poisoning/critical alerts. The 7 actions/untrusted-checkout/critical alerts on .github/actions/run-lint-and-unit-tests/action.yaml and .github/workflows/on-pr-ocr-onnx.yml are deferred to a follow-up PR (as discussed in the QVAC-18612 thread).

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Tier-based Approval Status

**PR Tier:** TIER1

**Current Status:** ✅ APPROVED

**Requirements:**
- 1 Team Member approval ❌ (0/1)
- 1 Team Lead OR Management approval ✅ (2/1)

**Bypass rule:** Triggered (2+ Team Lead approvals (Tier 1 exception)). This PR is approved regardless of tier.

---
*This comment is automatically updated when reviews change.*

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/ocr-onnx (iOS)

Result: passed

metric value
Devices passed 4
Devices failed 0
Test cases total 12
Test cases passed 12
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/translation-nmtcpp (iOS)

Result: passed

metric value
Devices passed 3
Devices failed 0
Test cases total 9
Test cases passed 9
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/ocr-onnx (Android)

Result: passed

metric value
Devices passed 6
Devices failed 0
Test cases total 18
Test cases passed 18
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/translation-nmtcpp (Android)

Result: passed

metric value
Devices passed 2
Devices failed 0
Test cases total 6
Test cases passed 6
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/llm-llamacpp (iOS)

Result: passed

metric value
Devices passed 26
Devices failed 0
Test cases total 78
Test cases passed 78
Test cases failed 0
Test cases skipped 0

View workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Mobile integration tests — @qvac/llm-llamacpp (Android)

Result: failed

metric value
Devices passed 8
Devices failed 1
Test cases total 27
Test cases passed 25
Test cases failed 0
Test cases skipped 1

View workflow run

@kinsta

kinsta Bot commented Jun 1, 2026

Copy link
Copy Markdown

Preview deployments for qvac-docs-staging ⚡️

Status Branch preview Commit preview
🔁 Deploying... N/A N/A

Commit: 38f4b9fec156b6825c98731250245657c87373a8

Deployment ID: 119bd056-677c-495a-8ec2-329601f9b025

Static site name: qvac-docs-staging-fazwv

@GSServita

Copy link
Copy Markdown
Contributor Author

/review

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

Labels

verified Authorize secrets / label-gate in PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants