Skip to content

fix: redact sensitive tokens from invoke CLI output#1419

Open
notgitika wants to merge 1 commit into
aws:mainfrom
notgitika:fix/invoke-redact-sensitive-output
Open

fix: redact sensitive tokens from invoke CLI output#1419
notgitika wants to merge 1 commit into
aws:mainfrom
notgitika:fix/invoke-redact-sensitive-output

Conversation

@notgitika
Copy link
Copy Markdown
Contributor

Summary

Restores redactSensitiveText() that was present on the preview branch but dropped during the preview→main consolidation (PR #1341).

Without this, bearer tokens and client secrets can appear verbatim in --json output or error messages when invoking harnesses with CUSTOM_JWT auth.

Redacts:

  • Bearer <token>Bearer [REDACTED]
  • client_secret=<value>client_secret=[REDACTED]
  • token=<value>token=[REDACTED]

Test plan

  • TypeScript compiles cleanly
  • Lint/prettier pass
  • Manual: invoke with --bearer-token + --json, verify token is redacted in output

Restores redactSensitiveText() that was present on the preview branch
but dropped during the preview→main consolidation. Prevents bearer
tokens and client secrets from appearing in --json output or error
messages printed to stderr.
@notgitika notgitika requested a review from a team May 28, 2026 20:56
@github-actions github-actions Bot added size/xs PR size: XS agentcore-harness-reviewing AgentCore Harness review in progress labels May 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

gh release download pr-1419-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.15.0.tgz

@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label May 28, 2026
@agentcore-devx-automation
Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label May 28, 2026
Copy link
Copy Markdown

@agentcore-cli-automation agentcore-cli-automation left a comment

Choose a reason for hiding this comment

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

Thanks for restoring this. The intent is right, but I think there are a couple of correctness gaps worth addressing before merge — primarily that the JSON-quoted forms of client_secret and token (which are the most common shapes when redacting from --json output and from agent/server response bodies) are not actually matched by the current regexes.

A second gap: errors thrown from inside the invoke flow that propagate up to the outer catch (error) in command.tsx (around lines 354–361) and the early --region is required exit inside handleInvokeCLI (around lines 52–58) bypass redactSensitiveText entirely. These paths can carry network error bodies (e.g. Invoke failed (${status}): ${body} from invokeWithBearerToken* in src/cli/aws/agentcore.ts), so they should also go through the redactor — either inline or via a shared helper.

return value
.replace(/(bearer\s+)[a-z0-9\-._~+/]+=*/gi, '$1[REDACTED]')
.replace(/(client[_-]?secret\s*[:=]\s*)([^,\s]+)/gi, '$1[REDACTED]')
.replace(/(token\s*[:=]\s*)([^,\s]+)/gi, '$1[REDACTED]');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Regexes don't match JSON-quoted forms — the most common shape this PR is meant to cover.

The client_secret and token patterns require secret / token to be followed (after optional whitespace) by a literal : or =. In JSON output (the PR's stated motivating case — --json), the actual shape is:

"client_secret":"abc123"
"token":"abc123"
"access_token":"jwt.token.here"

There's a " between the key and the :, so \s*[:=]\s* does not match and these values are not redacted. Quick check:

redact('{"client_secret":"abc123"}')
// => '{"client_secret":"abc123"}'   (unchanged)
redact('{"token":"abc123"}')
// => '{"token":"abc123"}'           (unchanged)

This means agent responses or upstream error bodies that echo headers/tokens as JSON (a realistic case for misbehaving harnesses or 4xx response bodies) still leak.

Options:

  1. Allow an optional " (or any non-word char) between the key and the separator, e.g. /(client[_-]?secret["']?\s*[:=]\s*["']?)([^"',\s}]+)/gi and similarly for token.
  2. Add separate JSON-shape patterns alongside the existing key=value / key: value ones.
  3. Redact the known-sensitive value directly when we have it (e.g., when options.bearerToken is set, do a literal replaceAll(options.bearerToken, '[REDACTED]') in addition to the heuristic regexes). This is the most reliable for the bearer case and is independent of the surrounding syntax.

Whichever path you take, please widen the value character class so the closing " / } aren't consumed as part of the redacted value (the current [^,\s]+ would swallow them when no comma/whitespace follows).

.replace(/(bearer\s+)[a-z0-9\-._~+/]+=*/gi, '$1[REDACTED]')
.replace(/(client[_-]?secret\s*[:=]\s*)([^,\s]+)/gi, '$1[REDACTED]')
.replace(/(token\s*[:=]\s*)([^,\s]+)/gi, '$1[REDACTED]');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No tests for redactSensitiveText.

This is a security-sensitive helper that's easy to get subtly wrong (see the JSON-shape comment above — a unit test would have caught it immediately). A small table-driven test in src/cli/commands/invoke/__tests__/ covering at least:

  • Bearer <jwt> in a plain string and inside a JSON-stringified body
  • client_secret=... and "client_secret":"..."
  • token=... and "access_token":"..."
  • A negative case (no sensitive content → unchanged)

would be enough to lock down the behavior and prevent regressions like the one this PR is fixing.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xs PR size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants