Skip to content

feat: onboarding dust-github-action#1

Open
Raj-StepSecurity wants to merge 3 commits into
mainfrom
release
Open

feat: onboarding dust-github-action#1
Raj-StepSecurity wants to merge 3 commits into
mainfrom
release

Conversation

@Raj-StepSecurity

Copy link
Copy Markdown
Collaborator

No description provided.

@Raj-StepSecurity Raj-StepSecurity added the review-required Request Claude AI code review on the PR label Jul 23, 2026
const { core } = config;

const zip = new AdmZip();
addDirectoryToZip(zip, ".", ".");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Security: Entire workspace sent to external API

addDirectoryToZip(zip, ".", ".") packages the entire repository (only skipping .git and node_modules) and sends it to Dust's API. Any secrets files present in the repo (.env, private keys, config files with credentials) would be included in the upload.

Consider adding an explicit allowlist (only include directories with SKILL.md) or at minimum a denylist that also skips common sensitive file patterns:

Suggested change
addDirectoryToZip(zip, ".", ".");
addDirectoryToZip(zip, ".", ".", [".env", "*.pem", "*.key", "*.p12", "*.pfx"]);

Or, better, only zip directories that contain a SKILL.md file rather than the entire workspace.

}

const files = patterns.flatMap((pattern) => {
const matches = fs.globSync(pattern);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Security: Path traversal risk via user-supplied glob patterns

fs.globSync(pattern) executes directly with patterns sourced from the agent-configs workflow input. A pattern such as ../../secrets or /etc/passwd could resolve files outside the workspace directory. The matched files are then parsed as YAML and their full content is sent (via PATCH/POST) to the Dust API — making this a potential exfiltration vector.

Add a guard to reject patterns that resolve outside the workspace:

const resolved = path.resolve(f);
const cwd = process.cwd();
if (!resolved.startsWith(cwd + path.sep) && resolved !== cwd) {
  throw new Error(`Pattern "${pattern}" matched file outside workspace: ${f}`);
}

Comment thread src/index.js
core.error(
`\u001b[31mLearn how to enable a subscription: ${docsUrl}\u001b[0m`
);
process.exit(1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-idiomatic failure pattern — use core.setFailed()

process.exit(1) bypasses any cleanup handlers registered in the Node.js process and suppresses the standard GitHub Actions error message formatting. Prefer the idiomatic approach:

Suggested change
process.exit(1);
core.setFailed(
`This action requires a StepSecurity subscription for private repositories. Learn how to enable a subscription: ${docsUrl}`
);
return;

test-upsert-skills:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suspicious action versions — checkout@v7 and setup-node@v7

Both actions/checkout and actions/setup-node were at v4 as of the most recent stable releases. @v7 does not exist and will cause this workflow to fail at runtime. Please pin to known stable versions:

Suggested change
- uses: actions/checkout@v7
- uses: actions/checkout@v4

steps:
- uses: actions/checkout@v7

- uses: actions/setup-node@v7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same issue — actions/setup-node@v7 does not exist:

Suggested change
- uses: actions/setup-node@v7
- uses: actions/setup-node@v4

@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based action — uses runs: using: "node24" in action.yml, compiled to dist/index.js via @vercel/ncc.


✅ Passed Checks

  • License: LICENSE is present with copyright for both Dust (original author) and StepSecurity.
  • action.yml author: Set to step-security
  • SECURITY.md: Present with vulnerability reporting instructions ✅
  • No FUNDING.yml: Not present ✅
  • .github/workflows required files: Both auto_cherry_pick.yml and actions_release.yml are present ✅
  • No renovate.json: Not present ✅
  • No PULL_REQUEST.md: Not present ✅
  • No ISSUE_TEMPLATE folder: Not present ✅
  • No CHANGELOG.md: Not present ✅
  • No .vscode folder: Not present ✅
  • README banner: StepSecurity maintained-action banner is present at the top of README.md
  • README usage examples: Use major-version-only tag (@v0), no full semver tags ✅
  • Subscription check URL: Correctly calls https://agent.api.stepsecurity.io/v1/github/$GITHUB_REPOSITORY/actions/maintained-actions-subscription
  • upstream variable: Value "dust-tt/dust-github-action" correctly matches original-owner: "dust-tt" + repo-name: "dust-github-action" from auto_cherry_pick.yml
  • package.json author: Set to "step-security"
  • package.json repository: URL contains step-security
  • dist/ folder: Present with compiled index.js
  • package.json has build script: Present (ncc build src/index.js ...) ✅

❌ Failed Checks

  • test.yml uses non-existent action versions: Both actions/checkout@v7 (lines 10, 44) and actions/setup-node@v7 (lines 12, 46) reference versions that do not exist — both actions are at v4. The test workflow will fail at runtime. Pin to @v4.

⚠️ Warnings

  • process.exit(1) in subscription check (src/index.js line 45): Using process.exit(1) directly bypasses Node.js cleanup handlers and suppresses standard GitHub Actions error formatting. Prefer core.setFailed(message); return; for idiomatic action failure handling.
  • API error bodies logged verbatim (src/utils/fetch.js lines 21, 33): Full API response bodies are included in thrown Error messages and thus in workflow logs. If the Dust API ever returns sensitive context in error responses, this could surface in public CI logs.

🔒 Security Findings

  1. Entire workspace uploaded to external API (src/methods/upsert-skills.js line 17): addDirectoryToZip(zip, ".", ".") packages everything in the repository (only skipping .git and node_modules) and sends it to dust.tt. Any .env files, private keys, or credentials present in the workspace would be included in the upload. The function should be scoped to only include directories containing a SKILL.md file, or at minimum explicitly deny sensitive file patterns.

  2. Path traversal via glob patterns (src/methods/upsert-agent-configs.js line 25): fs.globSync(pattern) executes directly with user-supplied patterns from the agent-configs workflow input. Patterns like ../../secrets or /etc/passwd can resolve files outside the workspace. Matched file contents are then sent to the Dust API via PATCH/POST, creating a potential data exfiltration path. Add a guard that rejects any resolved path falling outside process.cwd().


Summary

The action passes all StepSecurity onboarding checklist items except for non-existent action versions in test.yml. Two security issues of moderate-to-high severity need to be addressed before this action is safe for general use: the unrestricted workspace packaging in upsert-skills and the path traversal risk in the agent-configs glob resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-required Request Claude AI code review on the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant