🐛 ci: harden CI against SonarCloud vulnerability findings#358
Merged
Conversation
SonarCloud flagged 72 findings (githubactions:S8544/S8541) across the uv sync/run invocations in verify-python, verify-all-samples, and verify-scenarios: none asserted --locked against uv.lock or passed --no-build, so a stale lockfile or a malicious sdist build script could silently change what gets installed in CI. Adds --locked --no-build to every uv sync/uv run call, with one documented exception: python/semantic-kernel-tool-policy can't use --no-build because its transitive dep pybars4 ships no wheel on PyPI (sdist only). Also regenerates python/haystack-tool-policy/uv.lock, which was stale relative to its pyproject.toml constraint (>=2.31.0,<3.0 -> <4.0); no dependency versions changed, only the recorded constraint metadata.
SonarCloud flagged 6 findings (githubactions:S6505) in verify-node.yml: pnpm install ran without --ignore-scripts, letting a compromised dependency's lifecycle script (preinstall/install/postinstall) execute arbitrary code during CI. Verified none of the 6 node examples rely on a lifecycle script (no preinstall/install/postinstall/prepare entries in any package.json) and that install/typecheck/test still pass with the flag added.
SonarCloud flagged 3 findings (docker:S8482/S8541/S8544) in the live-core-enforcement python-agent Dockerfile: - line 15 piped curl straight to sh to install rustup with no verification of what was being executed (S8482); - line 24 installed maturin and agent-assembly unpinned and without --only-binary=:all:, allowing an sdist build script to run (S8541/S8544). Fixes: download rustup-init and its published sha256 digest separately, verify with sha256sum -c before chmod+exec (resolving the arch via TARGETARCH so both amd64 and arm64 runners work); pin maturin==1.14.1 and agent-assembly==0.0.1rc6 with --only-binary=:all: (both already ship wheels for this platform, so no build behavior changes). Verified end-to-end with a local docker build — the native agent_assembly._core extension still loads correctly in the built image.
This repo's SonarCloud project shows 6% duplicated lines (org threshold <3%), almost entirely from per-example policy.py/policy.ts/policy.go (plus a few main.py/index.ts/test_smoke.py) files that are deliberately duplicated across ~13 independent example subprojects — this is a tutorials repo where each example must be self-contained and copy-paste-able, so extracting a shared module would break that goal. Adds .sonarcloud.properties (not sonar-project.properties, which Automatic Analysis ignores) with sonar.cpd.exclusions listing the exact 30 duplicated files from the current duplication report — this property disallows wildcard patterns, so no repo has no sonar-scanner CI step; analysis runs via SonarCloud's Automatic Analysis, confirmed via the scanner context's sonar.autoscan.enabled=true.
The previous fix built the uv flag as $no_build_flag (empty string for semantic-kernel-tool-policy, "--no-build" otherwise) and interpolated it into the case statement. SonarCloud's static analysis can't follow that indirection and re-flagged the interpolated call sites as if --no-build were always missing (githubactions:S8541, re-triggered on this PR's diff). Split into two literal case statements instead — one for semantic-kernel-tool-policy (no --no-build, documented why) and one for everything else (always --no-build) — so every call site is a static, directly-scannable command.
discover_samples.py always classifies python/semantic-kernel-tool-policy as uv-pytest (it has a tests/ dir), so the uv-main/py-script cases inside its --no-build exception branch were unreachable — but SonarCloud's static analysis flagged them anyway (githubactions:S8541) since it scans every literal command, reachable or not. Collapses the exception to the one path that's actually taken, with a guard that fails loudly if discovery's classification ever changes.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--locked --no-buildto everyuv sync/uv runcall inverify-python.yml,verify-all-samples.yml, andverify-scenarios.yml(fixesgithubactions:S8544/S8541, 72 findings). One documented exception:python/semantic-kernel-tool-policykeeps source builds because its transitive deppybars4ships no wheel on PyPI.--ignore-scriptsto everypnpm installinverify-node.yml,verify-live.yml,verify-scenarios.yml,verify-all-samples.yml(fixesgithubactions:S6505, 10 findings). Verified no example relies on an install lifecycle script.--only-binary=:all:to the two barepip installcalls inverify-live.yml/verify-scenarios.yml(part of the S8541/S8544 family for pip).scenarios/live-core-enforcement/python-agent/Dockerfile(docker:S8482/S8541/S8544): verifies rustup's published sha256 before executing it instead of piping curl to sh, and pins+locks thepip install maturin agent-assemblystep with--only-binary=:all:..sonarcloud.propertieswithsonar.cpd.exclusionsfor the ~30 files wherepolicy.py/policy.ts/policy.go/main.py/test_smoke.pyare intentionally duplicated across independent, self-contained example subprojects (this repo's whole design point). This project uses SonarCloud Automatic Analysis, which ignoressonar-project.properties, so the exclusion goes in.sonarcloud.propertiesinstead (no wildcards allowed there, hence the explicit file list).python/haystack-tool-policy/uv.lock(was stale relative to itspyproject.tomlconstraint; no dependency version changes, only metadata).Why
Fixes the 85 open SonarCloud VULNERABILITY findings dragging this repo's Security rating to 5.0 (worst), plus brings duplicated-lines density reporting in line with intentional design (each example must stay copy-paste-able, so the org's <3% duplication threshold doesn't apply here without an exclusion).
Test plan
uv sync/uv run --locked --no-buildinvocation succeeds locally, one directory at a time (all 16python/*examples +scenarios/policy-enforcement/python+scenarios/approval-gates/python), including the--mock/src/main.pyentrypointspython/semantic-kernel-tool-policystill installs/tests without--no-buildand documented whynode/*pnpm examples plusscenarios/policy-enforcement/nodeandscenarios/approval-gates/nodeinstall/typecheck/test cleanly with--ignore-scriptspip install --only-binary=:all:commands resolve to wheels in an isolated venvTARGETARCH) and confirmed the nativeagent_assembly._coreextension still loads in the built image