Skip to content

Comments

Potential fix for code scanning alert no. 103: Artifact poisoning#336

Merged
Dargon789 merged 1 commit intomasterfrom
alert-autofix-103
Jan 16, 2026
Merged

Potential fix for code scanning alert no. 103: Artifact poisoning#336
Dargon789 merged 1 commit intomasterfrom
alert-autofix-103

Conversation

@Dargon789
Copy link
Owner

@Dargon789 Dargon789 commented Jan 16, 2026

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 ... Binary step to ensure that the computed PACKAGE_DIR is sane (no path traversal, exists, contains expected files) before invoking publish.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 }} Binary step (lines 221–241), enhance the shell script to:

  • Derive TOOL, PLATFORM, ARCH, and PACKAGE_DIR as before.
  • Reject unexpected values defensively (e.g., only allow a small, known set of characters in each, to prevent path manipulation).
  • Confirm that PACKAGE_DIR exists, is a directory, and resides strictly under ./@foundry-rs by resolving it to an absolute real path and checking it has the expected prefix.
  • Optionally, confirm that at least a minimal expected structure exists (e.g., that package.json and/or bin/ or a tool binary exists).
  • Only after successful validation, run bun ./scripts/publish.mjs "$PACKAGE_DIR".

These edits are localized to the script in the existing Publish ... Binary step in .github/workflows/npm.yml; no new imports are needed, since we rely on standard POSIX utilities (realpath, case pattern 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:

  • Add defensive validation for TOOL, PLATFORM, and ARCH values in the npm workflow to prevent malformed or malicious inputs.
  • Verify that the computed package directory exists, resides within the expected @foundry-rs root, and contains a package.json before invoking the publish script.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Dargon789 <[email protected]>
@codesandbox
Copy link

codesandbox bot commented Jan 16, 2026

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@gemini-code-assist
Copy link

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 16, 2026

Reviewer's Guide

Hardens 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 step

sequenceDiagram
    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
Loading

Flow diagram for PACKAGE_DIR validation and publish logic

flowchart 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]
Loading

File-Level Changes

Change Details Files
Add defensive validation for TOOL, PLATFORM, and ARCH values in the npm publish workflow shell step to prevent malformed inputs and path manipulation.
  • Introduce case-based checks that ensure TOOL, PLATFORM, and ARCH are non-empty and contain only alphanumeric characters, dashes, or underscores
  • Fail the job early with clear error messages if any matrix-derived value does not meet the allowed pattern
.github/workflows/npm.yml
Validate the computed PACKAGE_DIR and its location before publishing to ensure artifacts remain within the trusted directory hierarchy.
  • Compute PACKAGE_DIR from TOOL, PLATFORM, and ARCH as before, but log its value for traceability
  • Check that PACKAGE_DIR exists and is a directory before proceeding
  • Resolve PACKAGE_DIR and the expected @foundry-rs root via realpath and verify that the package path is strictly under the expected root, rejecting path traversal or directory escape attempts
.github/workflows/npm.yml
Add a minimal content sanity check for the package directory before running the publish script.
  • Require that package.json exists inside PACKAGE_DIR before allowing publication
  • Abort the publish step with an explicit error if the expected file is missing, preventing attempts to publish malformed or incomplete artifacts
.github/workflows/npm.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@snyk-io
Copy link

snyk-io bot commented Jan 16, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Repository owner deleted a comment from vercel bot Jan 16, 2026
@Dargon789 Dargon789 marked this pull request as ready for review January 16, 2026 04:06
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • 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.
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`.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Dargon789 Dargon789 merged commit 9c5e4e7 into master Jan 16, 2026
12 of 16 checks passed
@Dargon789 Dargon789 deleted the alert-autofix-103 branch January 16, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

# Sequence diagram for hardened npm publish workflow step # Sequence diagram for Docker image build and publish workflow

1 participant