Skip to content

test(security-review): same-repo path smoke test #1319

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): same-repo path smoke test #1319

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.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 inline-comment posting path. 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 embed an AWS access key ID and secret access key directly in source. Even though these are AWS's documented example values (AKIAIOSFODNN7EXAMPLE / wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY) and not real credentials, committing literals that match the AWS credential format:

  • Trips secret scanners (GitHub secret scanning, gitleaks, trufflehog) and pollutes their signal-to-noise.
  • Sets a bad pattern that can be copy-pasted into real code.
  • Is exactly the shape the /security-review skill is intended to flag.

This file should be removed before close. If a smoke test fixture is needed long-term, fetch credentials from process.env or a test-only secret manager rather than literals.

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

These constants embed an AWS access key ID and secret access key directly in the repository:

const HARDCODED_AWS_ACCESS_KEY_ID = 'AKIAIOSFODNN7EXAMPLE';
const HARDCODED_AWS_SECRET_ACCESS_KEY = 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY';

Even though these are AWS's documented "EXAMPLE" placeholder values (so no real account is exposed here), the pattern is dangerous: committing access-key-shaped strings to a repo trains contributors to hardcode credentials and bypasses secret-scanning heuristics that distinguish example vs. real keys mostly by regex. If a real AKIA… key followed the same pattern, anyone with read access to the repo (including via forks, GitHub mirrors, or git clone of stale checkouts) could exfiltrate it, and rotation requires revoking the IAM credential rather than just removing the file.

Remediation: Load credentials from the default AWS SDK credential provider chain (environment variables, shared config/credentials files, IAM role, SSO) — never inline them. For test fixtures that genuinely need a key-shaped string, use an obviously fake value (e.g., "FAKE-ACCESS-KEY") that won't match AKIA[0-9A-Z]{16} and add a .secretlintrc/gitleaks allowlist entry if the scanner flags it. Also ensure CI runs secretlint/gitleaks on every PR so a real key in this shape would block merge.


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';

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
.

@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 exec() with unvalidated user input.

target is read from the host query parameter on line 21 and interpolated directly into a shell command on line 24:

exec(`ping -c 1 ${target}`, ...)

A request like ?host=localhost;cat%20/etc/passwd (or `…`, $(…), &&, |, etc.) executes arbitrary shell commands as the server process. This is a textbook OS command injection (CWE-78).

Fixes (pick one):

  1. Use execFile('ping', ['-c', '1', target], …) so arguments are passed without a shell.
  2. Strictly validate target against an allowlist / hostname regex (e.g. /^[a-zA-Z0-9.-]+$/) and reject anything else before invoking exec.
  3. Drop the shell-out entirely and use a library/socket-based reachability check.

Again — this file is the smoke-test fixture and should be deleted after the workflow is verified rather than fixed in place.

@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 exec() with attacker-controlled input

target is read directly from the host query parameter and interpolated into a shell command string passed to child_process.exec:

const target = url.searchParams.get('host') ?? 'localhost';
exec(`ping -c 1 ${target}`, (err, stdout, stderr) => { ... });

exec invokes /bin/sh -c <string>, so any shell metacharacter in target is interpreted by the shell. A request like GET /?host=localhost;cat%20/etc/passwd runs an arbitrary follow-on command in the server process; $(…), backticks, &&, ||, |, redirection, and newlines are all available to the attacker. Because the stdout/stderr of the spawned shell is written back into the HTTP response, this is also a trivial data-exfiltration channel — the attacker reads the output of whatever command they injected.

Remediation: Use execFile (or spawn) with the binary and an argv array so arguments are passed to execve directly without a shell:

import { execFile } from 'node:child_process';
execFile('ping', ['-c', '1', target], (err, stdout, stderr) => { ... });

Additionally, validate target against an allow-list pattern before use (e.g., RFC 1123 hostname regex or net.isIP), reject anything else with 400, and consider rate-limiting / authentication on this endpoint since it lets unauthenticated callers trigger outbound network traffic from the server (SSRF-adjacent).

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

export { buildSignedRequest, server };
Loading