Skip to content

fix(eval): mark --header and --bearer-token sensitive in eval simulate - #2148

Merged
jariy17 merged 1 commit into
refactorfrom
follow-up/eval-simulate-sensitive-flags
Aug 31, 2026
Merged

fix(eval): mark --header and --bearer-token sensitive in eval simulate#2148
jariy17 merged 1 commit into
refactorfrom
follow-up/eval-simulate-sensitive-flags

Conversation

@jariy17

@jariy17 jariy17 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What

Follow-up to #2071. Mark --header and --bearer-token as sensitive on both eval ondemand simulate and eval batch-evaluation simulate, so the withLogging debug middleware redacts them instead of logging the CUSTOM_JWT bearer token and application headers in cleartext.

Addresses nborges-aws's review comment on #2071: "this and --header should be marked sensitive."

Why

The canonical runtime invoke handler already marks both flags sensitive (src/handlers/runtime/invoke/index.tsx). The two simulate twins accept the identical secret-carrying flags but omitted the annotation, so a --debug run would leak the token/headers. Fixed both twins for consistency — leaving the batch one would be half a fix for the same secret.

Already done in #2071 (not re-addressed here)

The other two review commitments landed before #2071 merged:

  • withUserCancellation shared helper — already used
  • expectedResponse reference inputs — already handled by toReferenceInputs

Test

bun test for both simulate handlers: 25 pass / 0 fail. bun run typecheck clean.

@github-actions github-actions Bot added the size/m PR size: M label Aug 31, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Aug 31, 2026
Follow-up to #2071. These flags carry secrets (CUSTOM_JWT bearer token,
auth headers) but were logged in cleartext by the withLogging debug
middleware. The canonical 'runtime invoke' handler already marks both
sensitive; apply the same to the on-demand and batch-evaluation simulate
handlers.
@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 Aug 31, 2026
@jariy17
jariy17 force-pushed the follow-up/eval-simulate-sensitive-flags branch from 8717cfd to 93de6fb Compare August 31, 2026 18:10
@github-actions github-actions Bot added size/xs PR size: XS and removed size/m PR size: M labels Aug 31, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

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.

AgentCore Harness Review

Verdict: Looks good

The functional change is small and correct: --header and --bearer-token in src/handlers/eval/batch-evaluation/simulate/index.tsx and src/handlers/eval/ondemand/simulate/index.tsx are now marked sensitive: true, matching the canonical runtime invoke handler and ensuring withLogging redacts these values.

One thing worth double-checking before merging — the bun.lock diff is much larger than the fix itself and includes changes that look unrelated:

  • lockfileVersion drops from 3 to 1, suggesting the lockfile was regenerated with an older Bun.
  • The explicit overrides entry for @aws-cdk/toolkit-libyaml: ^1 (added intentionally in commit 0565ca89 fix(deps): record the nested toolkit-lib yaml override in bun.lock) is removed from the lockfile's overrides block. The pin is still functionally in place because package.json retains the override and the resolved @aws-cdk/toolkit-lib/yaml → yaml@1.10.3 entry still appears in the lockfile, so this isn't a correctness bug — but it partly undoes what 0565ca89 was trying to record.
  • eslint bumps 9.39.5 → 10.9.1 (major) with a cascade of transitive dev-dep changes, which has nothing to do with marking flags sensitive.

Options for the author:

  1. Regenerate bun.lock on the same Bun version as main (keeping lockfileVersion: 3 and the explicit toolkit-lib override) so this PR only touches the two .tsx files.
  2. Or, if the lockfile drift is intentional, split it into a separate PR so this fix stays focused.

Not blocking on this if the team is comfortable with the lockfile churn.

Minor: other handlers (e.g. gateway/invoke/invoke.test.tsx) have a small test asserting flags().find(...).sensitive === true. Adding equivalent assertions here would prevent future regressions, but runtime/invoke doesn't have one either, so it's consistent as-is.

@agentcore-devx-automation agentcore-devx-automation Bot added claude-security-reviewing Claude Code /security-review in progress and removed agentcore-harness-reviewing AgentCore Harness review in progress labels Aug 31, 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 Aug 31, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (f431dce) to head (93de6fb).

Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2148   +/-   ##
=========================================
  Coverage     97.16%   97.16%           
=========================================
  Files           495      495           
  Lines         32676    32682    +6     
=========================================
+ Hits          31751    31757    +6     
  Misses          925      925           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jariy17
jariy17 merged commit 23714d3 into refactor Aug 31, 2026
22 checks passed
@jariy17
jariy17 deleted the follow-up/eval-simulate-sensitive-flags branch August 31, 2026 18:24
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.

3 participants