Potential fix for code scanning alert no. 103: Artifact poisoning#336
Merged
Potential fix for code scanning alert no. 103: Artifact poisoning#336
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Dargon789 <[email protected]>
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
Reviewer's GuideHardens the npm publish GitHub Actions workflow by validating matrix-derived package metadata and the computed package directory before invoking the publish script, mitigating artifact poisoning and path traversal risks. Sequence diagram for hardened npm publish workflow stepsequenceDiagram
actor Developer
participant GitHubActions as GitHub_Actions_Workflow
participant Job as Publish_Binary_Job
participant Shell as Publish_Step_Shell
participant FS as Filesystem_Artifacts
participant Script as publish_mjs
participant NPM as NPM_Registry
Developer->>GitHubActions: Push tag / trigger release
GitHubActions->>Job: Start matrix job (os, arch, tool)
Job->>Shell: Run Publish_${os}-${arch}_Binary step
Shell->>Shell: Set TOOL, PLATFORM, ARCH from matrix
Shell->>Shell: Validate TOOL value (allowed chars)
Shell->>Shell: Validate PLATFORM value (allowed chars)
Shell->>Shell: Validate ARCH value (allowed chars)
alt Invalid matrix value
Shell-->>Job: Exit 1 (invalid TOOL/PLATFORM/ARCH)
Job-->>GitHubActions: Mark job as failed
GitHubActions-->>Developer: Report workflow failure
else Matrix values valid
Shell->>Shell: Compute PACKAGE_DIR
Shell->>FS: Check PACKAGE_DIR exists and is directory
alt PACKAGE_DIR missing
Shell-->>Job: Exit 1 (missing PACKAGE_DIR)
Job-->>GitHubActions: Mark job as failed
else PACKAGE_DIR exists
Shell->>FS: Resolve ABS_PACKAGE_DIR via realpath
Shell->>FS: Resolve ABS_EXPECTED_ROOT for ./@foundry-rs
Shell->>Shell: Verify ABS_PACKAGE_DIR prefix is ABS_EXPECTED_ROOT
alt PACKAGE_DIR outside expected root
Shell-->>Job: Exit 1 (path traversal detected)
Job-->>GitHubActions: Mark job as failed
else PACKAGE_DIR within expected root
Shell->>FS: List contents of PACKAGE_DIR
Shell->>FS: Check PACKAGE_DIR/package.json exists
alt package.json missing
Shell-->>Job: Exit 1 (sanity check failed)
Job-->>GitHubActions: Mark job as failed
else package.json present
Shell->>Script: bun ./scripts/publish.mjs PACKAGE_DIR
Script->>NPM: Publish package
NPM-->>Script: Publish result
Script-->>Shell: Exit status
Shell-->>Job: Complete step
Job-->>GitHubActions: Job successful
GitHubActions-->>Developer: Report publish success
end
end
end
end
Flow diagram for PACKAGE_DIR validation and publish logicflowchart TD
A[Start Publish Binary step] --> B[Set TOOL from matrix.tool]
B --> C[Set PLATFORM from matrix.os]
C --> D[Set ARCH from matrix.arch]
D --> E{TOOL only has a-z A-Z 0-9 _ - and not empty}
E -->|No| Z1[Fail workflow: invalid TOOL] --> Z[End]
E -->|Yes| F{PLATFORM only has a-z A-Z 0-9 _ - and not empty}
F -->|No| Z2[Fail workflow: invalid PLATFORM] --> Z
F -->|Yes| G{ARCH only has a-z A-Z 0-9 _ - and not empty}
G -->|No| Z3[Fail workflow: invalid ARCH] --> Z
G -->|Yes| H[Compute PACKAGE_DIR = ./@foundry-rs/TOOL-PLATFORM-ARCH]
H --> I{PACKAGE_DIR exists and is directory}
I -->|No| Z4[Fail workflow: package directory does not exist] --> Z
I -->|Yes| J[ABS_PACKAGE_DIR = realpath PACKAGE_DIR]
J --> K[ABS_EXPECTED_ROOT = realpath ./@foundry-rs]
K --> L{ABS_PACKAGE_DIR starts with ABS_EXPECTED_ROOT/}
L -->|No| Z5[Fail workflow: directory outside expected root] --> Z
L -->|Yes| M[List PACKAGE_DIR contents with ls -la]
M --> N{PACKAGE_DIR/package.json exists}
N -->|No| Z6[Fail workflow: package.json missing] --> Z
N -->|Yes| O[Run bun ./scripts/publish.mjs PACKAGE_DIR]
O --> P[Echo Published @foundry-rs/TOOL-PLATFORM-ARCH]
P --> Z[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
casepatterns for validating TOOL/PLATFORM/ARCH use parentheses around the pattern list (e.g.(*[!a-zA-Z0-9_-]*|'')), which in POSIX shells/bASH are treated as literal characters; these should be rewritten to standardcasesyntax (e.g.*[!a-zA-Z0-9_-]*|'' )) so the validation actually triggers as intended. - The new
realpathusage assumes this utility is available on all runner OSes in the matrix; consider either checking for its presence or using a more portable alternative/fallback to avoid unexpected workflow failures on environments that lackrealpath.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `case` patterns for validating TOOL/PLATFORM/ARCH use parentheses around the pattern list (e.g. `(*[!a-zA-Z0-9_-]*|'')`), which in POSIX shells/bASH are treated as literal characters; these should be rewritten to standard `case` syntax (e.g. `*[!a-zA-Z0-9_-]*|'' )`) so the validation actually triggers as intended.
- The new `realpath` usage assumes this utility is available on all runner OSes in the matrix; consider either checking for its presence or using a more portable alternative/fallback to avoid unexpected workflow failures on environments that lack `realpath`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was
linked to
issues
Jan 16, 2026
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
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.
Potential fix for https://github.com/Dargon789/foundry/security/code-scanning/103
General fix approach: Ensure artifacts are treated as untrusted by (1) keeping them in an isolated directory (already done), and (2) validating any derived paths or contents before they are passed into scripts that may perform privileged actions (like publishing to npm). We should add explicit checks in the
Publish ... Binarystep to ensure that the computedPACKAGE_DIRis sane (no path traversal, exists, contains expected files) before invokingpublish.mjs. This keeps existing functionality while giving CodeQL a clear validation barrier.Concrete best fix for this snippet: In the
Publish ${{ matrix.os }}-${{ matrix.arch }} Binarystep (lines 221–241), enhance the shell script to:TOOL,PLATFORM,ARCH, andPACKAGE_DIRas before.PACKAGE_DIRexists, is a directory, and resides strictly under./@foundry-rsby resolving it to an absolute real path and checking it has the expected prefix.package.jsonand/orbin/or a tool binary exists).bun ./scripts/publish.mjs "$PACKAGE_DIR".These edits are localized to the script in the existing
Publish ... Binarystep in.github/workflows/npm.yml; no new imports are needed, since we rely on standard POSIX utilities (realpath,casepattern matches,test).Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by Sourcery
Harden the npm publish workflow against artifact path manipulation by validating matrix-derived parameters and package directories before publishing.
Bug Fixes: