-
Notifications
You must be signed in to change notification settings - Fork 575
Address critical Github-Security-Advisories, Add audit CI Check #3376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,33 @@ jobs: | |
| - run: npm run build | ||
| - run: npm run lint | ||
|
|
||
| audit: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 5 | ||
| strategy: | ||
| matrix: | ||
| node-version: [24.x] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ env.GIT_REF }} | ||
| fetch-depth: 0 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
Comment on lines
+91
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n .github/workflows/nodejs.yml | sed -n '80,110p'Repository: XRPLF/xrpl.js Length of output: 1145 🌐 Web query:
💡 Result: As of June 2026, pinning GitHub Actions to a full-length commit SHA is the recommended security industry standard to prevent supply chain attacks [1][2][3]. Key Security Principles 1. Immutability: Git tags (e.g., Citations:
🌐 Web query:
💡 Result: For the actions/setup-node GitHub Action, the security recommendation is to set package-manager-cache: false in workflows with elevated privileges or that access sensitive information [1][2][3]. This recommendation exists to mitigate the risk of cache poisoning [4][5]. When automatic dependency caching is enabled, an attacker who can influence the workflow could potentially inject malicious content into the build cache [5]. If this poisoned cache is subsequently used in a privileged context—such as a release workflow—it may lead to compromised build artifacts or the exposure of sensitive credentials (including OIDC tokens) to attacker-controlled code [4]. Key points regarding this recommendation: 1. Scope of Risk: The risk is most significant in privileged workflows where the cache might be read or written by processes with access to secrets or the ability to publish packages [4][6]. 2. Default Behavior: Automatic caching for npm is enabled by default if the package.json file includes a packageManager field set to npm (or devEngines.packageManager), unless an explicit cache input is provided [1][7][3]. 3. Implementation: You can explicitly disable this behavior by setting package-manager-cache: false in your workflow YAML configuration [1][8]. 4. Official Guidance: The official actions/setup-node documentation explicitly advises disabling automatic caching in sensitive or elevated contexts when caching is not strictly required for secure operation [1][2][3]. Additionally, it is noted that disabling caching is a standard practice when running workflows without a lockfile [8]. Citations:
Pin GitHub Actions by commit SHA and disable automatic package-manager caching. Lines 91 and 96 use mutable tags ( For SHA pinning, use the full commit hash with the version tag as a comment for maintainability (e.g., Suggested hardening patch- - uses: actions/checkout@v4
+ - uses: actions/checkout@<COMMIT_SHA> # v4
with:
ref: ${{ env.GIT_REF }}
fetch-depth: 0
- name: Use Node.js ${{ matrix.node-version }}
- uses: actions/setup-node@v4
+ uses: actions/setup-node@<COMMIT_SHA> # v4
with:
node-version: ${{ matrix.node-version }}
+ package-manager-cache: false🧰 Tools🪛 zizmor (1.25.2)[warning] 91-94: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) [error] 91-91: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [error] 96-96: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [error] 96-96: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default (cache-poisoning) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| - name: Setup npm version 10 | ||
| run: | | ||
| npm i -g npm@10 --registry=https://registry.npmjs.org | ||
|
|
||
| # Fail the build on any high or critical npm advisory. Moderate/low are | ||
| # reported but do not block. Runs from package-lock.json (no install | ||
| # needed). | ||
| - name: Audit dependencies (high and critical) | ||
| run: npm audit --audit-level=high --registry=https://registry.npmjs.org | ||
|
|
||
| unit: | ||
| if: ${{ github.event_name != 'workflow_dispatch' || inputs.run_unit_tests != false }} | ||
| runs-on: ubuntu-latest | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: XRPLF/xrpl.js
Length of output: 558
🌐 Web query:
GitHub Actions checkout action persist-credentials security best practices💡 Result:
In GitHub Actions, the
persist-credentialsinput for theactions/checkoutaction controls whether the action configures theGITHUB_TOKENor SSH key into the local Git configuration (.git/config) [1][2]. It defaults totrue[1][3]. Security Best Practices: 1. Always setpersist-credentials: falseunless your workflow specifically requires running authenticatedgitcommands (e.g.,git push,git commit) [3][4]. 2. Explicitly include this configuration in everyactions/checkoutstep, as omitting the line leaves it enabled by default [3]. 3. Be aware of the "ArtiPACKED" vulnerability: Ifpersist-credentialsistrueand you subsequently upload the workspace (or the.gitdirectory) as a workflow artifact, the sensitive credentials stored in.git/configmay be leaked into that artifact, becoming accessible to anyone with access to the workflow's artifacts [5][6][2]. If your workflow needs to perform authenticated Git operations: - Use theGITHUB_TOKENwith the minimal required permissions (e.g., setpermissions: contents: writeonly where necessary) [7][3]. - If you must persist credentials for other tools, ensure artifact upload steps are strictly scoped to exclude the.git/directory [8][4]. While recent versions ofactions/checkout(such as v6) have improved security by storing credentials in a separate file under$RUNNER_TEMPrather than directly in.git/config, settingpersist-credentials: falseremains the recommended best practice to adhere to the principle of least privilege and eliminate the risk of accidental credential exposure [1][9].Citations:
persist-credentialsor change the default tofalseactions/checkout#485persist-credentials=trueplease actions/checkout#2312🏁 Script executed:
cat .github/workflows/nodejs.yml | tail -n +90Repository: XRPLF/xrpl.js
Length of output: 5427
Add
persist-credentials: falseto allactions/checkoutsteps.The workflow contains multiple
actions/checkoutsteps (lines 91-94, 126-129, 170-173, 219-222) that do not explicitly disable credential persistence. Since none of the subsequent steps perform authenticated git operations, credentials should not be persisted to follow the principle of least privilege and mitigate the risk of token exposure through artifacts (ArtiPACKED vulnerability).Suggested hardening patch
- uses: actions/checkout@v4 with: ref: ${{ env.GIT_REF }} fetch-depth: 0 + persist-credentials: falseApply this change to all four checkout steps in the workflow.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 91-94: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Source: Linters/SAST tools