Uh oh!
There was an error while loading. Please reload this page.
fix(eval): mark --header and --bearer-token sensitive in eval simulate - #2148
Conversation
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.
Claude Security Review: no high-confidence findings. (run) |
8717cfd to
93de6fbCompareThere was a problem hiding this comment.
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:
lockfileVersiondrops from3to1, suggesting the lockfile was regenerated with an older Bun.- The explicit
overridesentry for@aws-cdk/toolkit-lib→yaml: ^1(added intentionally in commit0565ca89 fix(deps): record the nested toolkit-lib yaml override in bun.lock) is removed from the lockfile'soverridesblock. The pin is still functionally in place becausepackage.jsonretains the override and the resolved@aws-cdk/toolkit-lib/yaml → yaml@1.10.3entry still appears in the lockfile, so this isn't a correctness bug — but it partly undoes what0565ca89was trying to record. eslintbumps 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:
- Regenerate
bun.lockon the same Bun version asmain(keepinglockfileVersion: 3and the explicittoolkit-liboverride) so this PR only touches the two.tsxfiles. - 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.
Claude Security Review: no high-confidence findings. (run) |
codecov-commenter
commented
Aug 31, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
What
Follow-up to #2071. Mark
--headerand--bearer-tokenassensitiveon botheval ondemand simulateandeval batch-evaluation simulate, so thewithLoggingdebug 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
--headershould be marked sensitive."Why
The canonical
runtime invokehandler already marks both flags sensitive (src/handlers/runtime/invoke/index.tsx). The twosimulatetwins accept the identical secret-carrying flags but omitted the annotation, so a--debugrun 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:
withUserCancellationshared helper — already usedexpectedResponsereference inputs — already handled bytoReferenceInputsTest
bun testfor both simulate handlers: 25 pass / 0 fail.bun run typecheckclean.