Skip to content

Address critical Github-Security-Advisories, Add audit CI Check#3376

Open
ckeshava wants to merge 3 commits into
XRPLF:mainfrom
ckeshava:updateDepsJune2026
Open

Address critical Github-Security-Advisories, Add audit CI Check#3376
ckeshava wants to merge 3 commits into
XRPLF:mainfrom
ckeshava:updateDepsJune2026

Conversation

@ckeshava

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

  • This PR upgrades dependencies of the xrpl.js monorepo via the npm audit fix command.
  • A new CI check is introduced. This check ensures that an outstanding "high/critical" vulnerability is not cleared by the CI.
  • The changes in the PR address 4 high, 1 critical vulnerability in the dependencies

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release
  • Upgrade dependencies

Did you update HISTORY.md?

  • Yes
  • No, this change does not impact library users

Test Plan

Existing tests pass, no functional change in the APIs / methods of the SDK.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

A new audit CI job is added to the GitHub Actions workflow, running npm audit --audit-level=high on Node.js 24.x after build-and-lint completes. Two transitive dependencies, tmp and form-data, are pinned to patched versions via an overrides block in package.json.

Changes

Security Audit CI and Dependency Overrides

Layer / File(s) Summary
Transitive dependency version overrides
package.json
Adds an overrides block pinning tmp to 0.2.7 and form-data to 4.0.6, with an inline metadata comment documenting the security rationale and intended removal timing.
npm audit CI job
.github/workflows/nodejs.yml
Inserts an audit job that runs after build-and-lint on Node.js 24.x, installs npm v10, and executes npm audit --audit-level=high against package-lock.json without a full dependency install step.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • XRPLF/xrpl.js#2695: Modifies the same .github/workflows/nodejs.yml to add Node 22.x support and bump npm to v10, directly intersecting with the audit job's npm v10 setup step.
  • XRPLF/xrpl.js#3000: Also edits .github/workflows/nodejs.yml to change Node.js version coverage and job triggering logic, overlapping with the job structure modified here.

Suggested reviewers

  • justinr1234
  • achowdhry-ripple
  • pdp2121

Poem

🐇 Hop, hop, audit time!
Two deps are pinned so finely,
tmp and form-data patched divinely.
A new CI job watches with care,
High advisories? It'll stop right there!
The warren stays secure — I declare! 🔒

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main changes: addressing GitHub Security Advisories and adding an audit CI check, both of which are core to this PR.
Description check ✅ Passed The description covers the high-level overview, type of change (marked as Upgrade dependencies), and test plan. However, the Context of Change section is missing entirely.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/nodejs.yml:
- Around line 92-94: The workflow contains multiple actions/checkout steps that
do not explicitly disable credential persistence, which creates a security risk.
Add persist-credentials: false as a parameter to each of the four
actions/checkout step occurrences in the workflow to prevent git credentials
from being persisted to disk, since none of the subsequent steps perform
authenticated git operations. This follows the principle of least privilege and
mitigates token exposure risk from artifacts.
- Around line 91-98: Replace the mutable version tags on the `actions/checkout`
and `actions/setup-node` actions with full commit SHAs, adding the original
version tag as a comment for maintainability (format:
`actions/name@<full-commit-sha> # <version-tag>`). Additionally, add
`package-manager-cache: false` to the `actions/setup-node` step configuration to
disable automatic package-manager caching and mitigate cache-poisoning exposure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 64f36104-3469-4b0f-a349-dba0075a97d5

📥 Commits

Reviewing files that changed from the base of the PR and between 7791ce2 and dcbb720.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • .github/workflows/nodejs.yml
  • package.json

Comment on lines +91 to +98
- 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 }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/nodejs.yml | sed -n '80,110p'

Repository: XRPLF/xrpl.js

Length of output: 1145


🌐 Web query:

GitHub Actions security best practices pinning commit SHA vs tags 2025

💡 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., @v4) are mutable references that can be deleted, moved, or recreated by a repository owner to point to a different commit [4][5]. If an action's repository is compromised, an attacker can retarget existing tags to point to malicious code [2][6]. Pinning to a full-length commit SHA provides an immutable reference that guarantees your workflow runs exactly the code you reviewed [4][6][7]. 2. Enforcement: Organizations can now enforce SHA pinning at the enterprise, organization, or repository level [1][2][3]. When enabled, any workflow referencing an action by a mutable tag or branch will fail [1][4]. 3. Recommended Practice: Use full-length commit SHAs for all third-party actions [2][8]. To maintain readability and ease of updates, include the version tag as a comment in your YAML file [2][6]: - uses: actions/checkout@3df4ab1 # v4.0.0 Automation and Maintenance Because pinned SHAs require manual updates to receive fixes, automation is essential for both security and maintenance [2][5]: - Use tools like Renovate or Dependabot to automatically generate pull requests when a new version tag is released, keeping your pinned SHAs up to date [1][2][5]. - When configuring automated tools, ensure they are set to pull updates for review rather than automatically merging, as an attacker could still publish a new, malicious version tag that your automation might otherwise adopt [6][5]. - Tools such as zizmor or CodeQL can be used to audit your workflows and flag any actions that are not currently pinned to a commit SHA [2][8]. While pinning to a tag is more convenient, it should only be considered for internal or highly trusted sources where you accept the risk of mutable references [3][9]. In all other cases, particularly with third-party dependencies, SHA pinning is the primary defense against tag-retargeting attacks [2][6][5].

Citations:


🌐 Web query:

actions/setup-node package-manager-cache false security recommendation

💡 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 (@v4), which weakens supply-chain integrity by allowing tag-retargeting attacks if the action repository is compromised. Also, set package-manager-cache: false on actions/setup-node to mitigate cache-poisoning exposure in the audit workflow.

For SHA pinning, use the full commit hash with the version tag as a comment for maintainability (e.g., actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0). Update these pinned SHAs regularly using automation tools like Dependabot or Renovate.

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 Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/nodejs.yml around lines 91 - 98, Replace the mutable
version tags on the `actions/checkout` and `actions/setup-node` actions with
full commit SHAs, adding the original version tag as a comment for
maintainability (format: `actions/name@<full-commit-sha> # <version-tag>`).
Additionally, add `package-manager-cache: false` to the `actions/setup-node`
step configuration to disable automatic package-manager caching and mitigate
cache-poisoning exposure.

Source: Linters/SAST tools

Comment on lines +92 to +94
with:
ref: ${{ env.GIT_REF }}
fetch-depth: 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/nodejs.yml | sed -n '85,100p'

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-credentials input for the actions/checkout action controls whether the action configures the GITHUB_TOKEN or SSH key into the local Git configuration (.git/config) [1][2]. It defaults to true [1][3]. Security Best Practices: 1. Always set persist-credentials: false unless your workflow specifically requires running authenticated git commands (e.g., git push, git commit) [3][4]. 2. Explicitly include this configuration in every actions/checkout step, as omitting the line leaves it enabled by default [3]. 3. Be aware of the "ArtiPACKED" vulnerability: If persist-credentials is true and you subsequently upload the workspace (or the .git directory) as a workflow artifact, the sensitive credentials stored in .git/config may 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 the GITHUB_TOKEN with the minimal required permissions (e.g., set permissions: contents: write only 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 of actions/checkout (such as v6) have improved security by storing credentials in a separate file under $RUNNER_TEMP rather than directly in .git/config, setting persist-credentials: false remains the recommended best practice to adhere to the principle of least privilege and eliminate the risk of accidental credential exposure [1][9].

Citations:


🏁 Script executed:

cat .github/workflows/nodejs.yml | tail -n +90

Repository: XRPLF/xrpl.js

Length of output: 5427


Add persist-credentials: false to all actions/checkout steps.

The workflow contains multiple actions/checkout steps (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: false

Apply 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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/nodejs.yml around lines 92 - 94, The workflow contains
multiple actions/checkout steps that do not explicitly disable credential
persistence, which creates a security risk. Add persist-credentials: false as a
parameter to each of the four actions/checkout step occurrences in the workflow
to prevent git credentials from being persisted to disk, since none of the
subsequent steps perform authenticated git operations. This follows the
principle of least privilege and mitigates token exposure risk from artifacts.

Source: Linters/SAST tools

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.

1 participant