Security changes - #359
Conversation
…esponses
SDK setup tools (setupBrowserStackAutomateTests and the Percy SDK flows)
interpolated the caller's real BROWSERSTACK_USERNAME/ACCESS_KEY into the
browserstack.yml and setup instructions returned to the LLM, so keys could
land in chat history, logs, or committed config files. Emit env-var
references (${BROWSERSTACK_USERNAME}/${BROWSERSTACK_ACCESS_KEY}) for
shell/yml and <your_browserstack_*> placeholders for JSON configs instead,
and stop threading the real credentials through the instruction call chain.
Also redact the stored site password in getAccessibilityAuthConfig's
response (mirrors the existing '***' redaction on the create path).
Adds regression tests asserting env-var references are emitted (never the
real credentials) and that the accessibility auth-config password is
redacted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
setupBrowserStackAppAutomateTests interpolated the caller's real BROWSERSTACK_USERNAME/ACCESS_KEY into the generated browserstack.yml, env exports, SDK setup commands, and language config files (Java, Python, C#, Node, Ruby) returned to the LLM — same exposure class as the web Automate setup tools. Emit <your_browserstack_*> placeholders instead and stop threading real credentials through the instruction generators. The credentials are still passed to uploadApp, which needs them for the actual app-upload API call (that path returns only a bs:// URL, no secrets). Adds a regression test covering every supported language/framework combo that asserts placeholders are emitted and real credentials never appear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SavioBS629
left a comment
SavioBS629
left a comment
There was a problem hiding this comment.
Claude Code Review (automated) — 9 inline finding(s). Full report in the PR comment below. Verdict: Failed - see PR comment.
Claude Code PR ReviewPR: #359 • Head: 01a8c56 • Reviewers: stack:code-review SummaryRemoves live BrowserStack credentials from generated SDK setup instructions across the Automate/App Automate/Percy flows — dropping The security intent is right and the signature refactor is mechanically complete (every call site updated; Review Table
Findings1.
|
| Artifact | sdk-utils (web) |
appium-sdk (app) |
|---|---|---|
browserstack.yml |
configUtils.ts:25 → ${BROWSERSTACK_USERNAME} |
config-generator.ts:38 → <your_browserstack_username> |
SDK setup CLI |
constants.ts:49,149,218 → ${...} |
csharp.ts:69,73, python.ts:86 → <your_...> |
Maven -D flags |
commands.ts:51 → ${...} |
java.ts:89,119 → <your_...> |
One of the two is wrong for any given consumer.
Suggestion: Pick one convention and centralise it in the new USERNAME_PLACEHOLDER / ACCESS_KEY_PLACEHOLDER constants. Since the appium flows do emit export steps via createEnvStep, ${BROWSERSTACK_USERNAME} is the coherent choice there and matches the SDK's documented browserstack.yml interpolation.
9. Dead parameter left behind
- File:
src/tools/sdk-utils/bstack/sdkHandler.ts:17 - Severity: Low
- Reviewer: stack:code-review
- Issue:
config: BrowserStackConfigis no longer referenced anywhere inrunBstackSDKOnly's body — its only former use was the deletedgetBrowserStackAuthcall. ESLint misses it becauseno-unused-varsdefaults toargs: "after-used"and the laterisPercyAutomateparam is still used. Its siblingrunPercyWithBrowserstackSDKdid drop the param, so the two handlers are now inconsistent. - Suggestion: Drop it and update
handler.ts:34andhandler.ts:129, or add a comment stating it is retained for signature stability.
10. The ruby.ts comment is misleading, and the import cycle should just be broken
- File:
src/tools/appautomate-utils/appium-sdk/languages/ruby.ts:14 - Severity: Low
- Reviewer: stack:code-review
- Issue: The TDZ hazard is real but newly introduced by this PR — the old top-level
const username = "${process.env.BROWSERSTACK_USERNAME}"was a plain string literal with no dependency on the barrel. The cycle islanguages/ruby.ts→../index.js→./instructions.js(index.ts:2) →./languages/ruby.js(instructions.ts:11), withexport * from "./constants.js"at index.ts:9 sitting after the instructions subgraph. Also, "Read lazily" is inaccurate — nothing is read; these are module constants. - Suggestion: Import directly from
../constants.jsinstead of the barrel. That removes the cycle entirely and lets the consts return to module scope.csharp.ts:7-8,java.ts:7-8,python.ts:9-10route through the barrel too and are safe only because they're referenced inside function bodies — a latent trap for the next editor.
11. Test env mutation without restore
- File:
tests/tools/sdk-utils-commands.test.ts:17 - Severity: Low
- Reviewer: stack:code-review
- Issue:
beforeEachsetsprocess.env.BROWSERSTACK_USERNAME/BROWSERSTACK_ACCESS_KEYto decoys and never restores them — the existingafterEach(:7-11) only restoresprocess.platform. The prior version deleted the vars (also unrestored, but benign); this version leaves fake credentials in the worker env for whatever runs after it. - Suggestion: Mirror the
afterEachrestore intests/tools/appautomate-sdk-credentials.test.ts:23-35, which gets this right.
12. Both new/rewritten test files under-assert
- File:
tests/tools/appautomate-sdk-credentials.test.ts:39 - Severity: Low
- Reviewer: stack:code-review
- Issue: The
not.toContain(DECOY_*)guards can only fail if an emitter readsprocess.envdirectly, which none of them ever did. The regression these tests should catch is re-introducing ausername/accessKeyparameter fed fromgetBrowserStackAuth(config)— nothing covers that. Worse, the 17-combo loop asserts nothing about content, so it would pass green if every generator returned"". - Suggestion: Add a positive assertion inside the loop (
toContain(USERNAME_PLACEHOLDER)under the chosen convention). All 17 combos return non-empty text today (prefix commands 203–2650 chars, project instructions 66–1292 chars), so this is safe to add.
Informational (non-gating)
- Removed auth validation is a real behavior change.
get-auth.ts:6-8throws when credentials are absent. DroppinggetBrowserStackAuth(config)frombstack/sdkHandler.tsandpercy-bstack/handler.tsremoves an implicit fail-fast, sorunTestsOnBrowserStack/setUpPercyHandler(AUTOMATE branch) will now render a full setup guide on a server with no credentials configured instead of erroring. Probably intended — but the PR is titled "Security changes" with an empty body, which also misses the conventional-commit and PR guidance in.claude/rules/commit-conventions.md. Please state this in the description. src/tools/testmanagement-utils/rich-text.ts:1is trailing-whitespace-only inside a comment. Confirmed inert — theTM_ALLOWED_TAGregex and all logic are byte-identical.- The accessibility redaction is type-safe.
AuthConfigResponse.datais a flat object (auth-config.ts:5-16), not the array the old test mocked, so{...result.data}cannot degrade into numeric keys. The updated mock intests/tools/accessibility.test.ts:41-47now matches the declared type — an improvement. - Env-var discipline holds. The
.claude/rules/security.mdcarve-out forsrc/tools/*/appium-sdk/languages/*.tsemitting literalprocess.env.BROWSERSTACK_*still applies tonodejs.ts:96-97; no new violation. - Pre-existing, untouched but inside a modified hunk:
appium-sdk/languages/csharp.ts:60usesconsole.warninstead oflogger.
Verdict: FAIL — the credential-removal goal is right, but the substituted placeholders produce shell blocks that do not parse, and the Python/C# flows now supply no credentials at all.
…-auth password
Follow-up to the credential-removal changes, resolving the automated PR
review findings:
- Use one consistent, quoted placeholder convention (<your_browserstack_*>)
everywhere. The previous mix of ${BROWSERSTACK_USERNAME} env-refs and bare
<placeholder> values produced shell that did not parse: `export VAR=<x>` is
a redirection error in bash, `<` is reserved in PowerShell, and cmd treats
it as redirection. All export/set/$env/CLI/yml/Maven spots now emit quoted
placeholders the user fills in — valid across bash, cmd, and PowerShell, and
removing the Python/C# flows' reliance on env vars nothing set.
- Redact the site password on the accessibility CREATE path too (not just
GET), plus the internal response logging, via a shared
redactAuthConfigResponse() helper.
- Break the appium-sdk import cycle at the source: constants.ts imports enums
from types.js instead of the index barrel, so ruby.ts can read the
placeholder constants at module scope again (removes the TDZ workaround).
- Document the retained (now-unused) config param in runBstackSDKOnly.
- Tests: restore mutated env in afterEach; add positive assertions so the
suites cannot pass green on empty output; cover the create-path redaction.
Verified: extracted and parsed every emitted bash block (63) — no syntax
errors from credential lines; 0 unquoted env-set placeholders remain.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SavioBS629
left a comment
SavioBS629
left a comment
There was a problem hiding this comment.
Claude Code Review (automated) — 8 inline finding(s). Full report in the PR comment below. Verdict: Passed.
| Run the following command to setup browserstack sdk: | ||
| \`\`\`bash | ||
| npx browserstack-node-sdk setup --username ${username} --key ${accessKey} | ||
| npx browserstack-node-sdk setup --username "<your_browserstack_username>" --key "<your_browserstack_access_key>" |
There was a problem hiding this comment.
[Medium] Nothing tells the user to substitute the placeholder
Across all 72 rendered credential-bearing blocks there is no substitution guidance — confirmed by grep, the only "Replace" text in the emitted output is configUtils.ts:28's TODO about projectName/buildName.
The consumer here is usually a coding agent, and these steps are prefixed "DO NOT SKIP ANY STEP … Each step is compulsory". The agent will run this line verbatim — and browserstack-node-sdk setup writes those values into browserstack.yml, baking the placeholder into project config. Tests then 401 at run time with nothing pointing at the cause. That trades the previous round's loud syntax error for a silent misconfiguration.
Suggestion: either append one line to each env/setup step — "Replace <your_browserstack_username> and <your_browserstack_access_key> with the values from https://www.browserstack.com/accounts/profile/details" — or put the export step first (properly fixing the earlier ordering finding) and reference "$BROWSERSTACK_USERNAME" / %BROWSERSTACK_USERNAME% / $env:BROWSERSTACK_USERNAME per shell.
Reviewer: stack:code-review
| * Redacts the stored site password before an auth-config response is returned | ||
| * to the LLM/tool caller or written to logs. Never expose the raw password. | ||
| */ | ||
| export function redactAuthConfigResponse( |
There was a problem hiding this comment.
[Medium] Redaction is shallow and single-key while callers stringify the whole object
This helper rewrites only the top-level data.password, but both callers (accessibility.ts:294-298 and :337-341) JSON.stringify the entire data object. The declared AuthConfigResponse.data includes username — the customer's site login — alongside password, so the site username is still echoed to the MCP client, and any key the API returns beyond the nine declared ones is dumped with no TypeScript warning.
The create request nests site credentials under authData: { username, password, ... } (:89-93, :149-153). If the response echoes that shape, the early return if (!response?.data?.password) never fires and the raw password passes straight through.
Suggestion: stop dumping the object — emit an explicit allowlist ({ id, name, type, url }), which also satisfies rules/tool-design.md § "Response payloads: trim before returning, never echo". If a generic helper is preferred, make it recurse, match on key name (/pass(word)?|secret|token|accesskey/i), and handle arrays and null explicitly.
Reviewer: stack:code-review
| `\`\`\`bash | ||
| export BROWSERSTACK_USERNAME=${username} | ||
| export BROWSERSTACK_ACCESS_KEY=${accessKey} | ||
| export BROWSERSTACK_USERNAME="<your_browserstack_username>" |
There was a problem hiding this comment.
[Low] Placeholder duplicated as a bare literal
USERNAME_PLACEHOLDER / ACCESS_KEY_PLACEHOLDER exist in appium-sdk/constants.ts:28-29, but this file hardcodes the literal string at :80,81,96,97,155,156,243,244,262,263,281,282, and all 25 occurrences under sdk-utils/bstack/ are bare literals too — ~34 sites total.
The values agree today, which is exactly how the convention divergence flagged in the previous review arose in the first place.
Suggestion: promote the two constants to a module both trees import (e.g. src/tools/sdk-utils/common/) and reference them everywhere.
Reviewer: stack:code-review
| if (originalPlatform) { | ||
| Object.defineProperty(process, "platform", originalPlatform); | ||
| } | ||
| process.env.BROWSERSTACK_USERNAME = originalUser; |
There was a problem hiding this comment.
[Low] Env restore writes the string "undefined"
Same issue as tests/tools/appautomate-sdk-credentials.test.ts:33 — assigning an undefined original coerces to the string "undefined" rather than removing the key, so teardown leaves a bogus value behind.
Suggestion: guard the restore, or use vi.stubEnv() / vi.unstubAllEnvs().
Reviewer: stack:code-review
| } from "../../src/tools/appautomate-utils/appium-sdk/index"; | ||
|
|
||
| // Every supported App Automate language/testing-framework combination. | ||
| const COMBOS: Array<[string, string[]]> = [ |
There was a problem hiding this comment.
[Low] Test omits the C# branch and the access-key placeholder
COMBOS covers java/python/nodejs/ruby but not csharp, so getCSharpSDKCommand() — one of the branches that embeds the placeholder into a dotnet browserstack-sdk setup command (csharp.ts:66,71) — is entirely uncovered by this suite.
The loop also asserts only <your_browserstack_username>; the standalone yml test at :82-96 correctly asserts both placeholders.
Suggestion: add ["csharp", ["nunit", "mstest", "xunit", "specflow", "reqnroll"]] to COMBOS, and assert <your_browserstack_access_key> inside the loop as well.
Reviewer: stack:code-review
| id: "auth-1", | ||
| name: "test", | ||
| username: "site-user", | ||
| password: "super-secret-site-password", |
There was a problem hiding this comment.
[Low] GET mock reshaped from array to object — confirm the real payload
This mock changed from data: [{ id, name }] to a flat object. That matches the declared AuthConfigResponse.data type and is defensible — but the previous mock asserted a list shape.
If the live endpoint can return a list, response.data.password is undefined, redactAuthConfigResponse returns the payload untouched, and this test passes green while production leaks the password.
Suggestion: confirm the endpoint's actual response shape. If a list is possible, handle it — the allowlist approach suggested on auth-config.ts:24 covers this case for free.
Reviewer: stack:code-review
| config: BrowserStackConfig, | ||
| isPercyAutomate = false, | ||
| ): Promise<RunTestsInstructionResult> { | ||
| void config; |
There was a problem hiding this comment.
[Low] Signature asymmetry, and breaking exports for the downstream wrapper
void config; keeps the unused param alive here, while the sibling runPercyWithBrowserstackSDK (percy-bstack/handler.ts:17) deleted its own config param outright — two conventions for the same situation.
More consequentially: per CLAUDE.md this package is consumed as a library by the remote MCP wrapper, and this PR removes positional params from exported symbols (getSDKPrefixCommand, getAppSDKPrefixCommand, generateBrowserStackYMLInstructions, generateAppBrowserStackYMLInstructions, getInstructionsForProjectConfiguration, formatEnvCommands, createEnvStep, runPercyWithBrowserstackSDK) plus the exported ConfigMapping.instructions type (common/types.ts:64). Because the removals are positional, a stale caller can silently pass username where appPath is now expected instead of failing to compile. In-repo callers are updated and tsc is green, but the wrapper builds separately.
Suggestion: pick one convention for the unused param, and call the export changes out in the PR body so the wrapper is updated in lockstep.
Reviewer: stack:code-review
Claude Code PR ReviewPR: #359 • Head: 04a143d • Reviewers: stack:code-review Continues the previous review — changes since SummaryRemoves live BrowserStack credentials from generated SDK setup instructions across the Automate/App Automate/Percy flows and redacts the stored site password in the accessibility auth-config tool responses. The new commit All six High findings from the previous round are genuinely fixed — verified mechanically, not by inspection. Review Table
FindingsResolved since
|
…stitution guidance
Follow-up to the previous review round (verdict PASS; these resolve the
remaining Medium/Low items):
- Auth-config responses now return an explicit allowlist ({id, name, type,
url}) instead of stringifying the whole data object, so the stored site
username (and any undeclared response key) is no longer echoed to the
caller — not just the password (N1). The internal redactor now masks
username too, and the form-auth error log no longer dumps the response
body (N3).
- Generated setup instructions now begin with an explicit step telling the
reader to replace <your_browserstack_*> with real credentials, so a coding
agent running steps verbatim doesn't bake the placeholder into
browserstack.yml and hit a silent 401 (N2).
- Placeholders now have a single source of truth in
sdk-utils/common/credentials.ts; appium-sdk/constants.ts re-exports from it
so the two trees cannot drift (N4).
- Tests: restore env with delete-when-undefined instead of the string
"undefined" (N5); cover the C# SDK branch and assert the access-key
placeholder too (N6).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SavioBS629
left a comment
SavioBS629
left a comment
There was a problem hiding this comment.
Claude Code Review (automated) — 7 inline finding(s). Full report in the PR comment below. Verdict: Passed.
| const data = response.data; | ||
| logger.info(`The data returned from the API is: ${JSON.stringify(data)}`); | ||
| logger.info( | ||
| `The data returned from the API is: ${JSON.stringify( |
There was a problem hiding this comment.
[Medium] logger.info still dumps the nested response unredacted
The error path was converted to status-only and the tool responses to a proper allowlist, but this info path still stringifies the whole response through redactAuthConfigResponse — which masks only top-level data.password / data.username (:24-34).
The request body nests credentials as authData: { username, password, ...selectors } (:107-111) — exactly the echo shape flagged last round. If the API echoes it, data.authData.password is logged verbatim, as are the selectors at any depth and any undeclared key.
Failure scenario: createFormAuthConfig("login", { username: "qa@acme.com", password: "S3cret!", ... }); endpoint returns {success:true, data:{id:7, name:"login", type:"form", authData:{username:"qa@acme.com", password:"S3cret!"}}} → the log line contains S3cret!, against .claude/rules/security.md "Never log credentials".
Suggestion: reuse the allowlist here — logger.info(\Auth config created: ${JSON.stringify(safeAuthConfigData(data))}`)— or logdata.idalone.redactAuthConfigResponse` then has no callers and can be deleted.
Reviewer: stack:code-review
| `\`\`\`bash | ||
| export BROWSERSTACK_USERNAME=${username} | ||
| export BROWSERSTACK_ACCESS_KEY=${accessKey} | ||
| export BROWSERSTACK_USERNAME="<your_browserstack_username>" |
There was a problem hiding this comment.
[Low] Bare placeholder literals survive the shared-module fix
src/tools/sdk-utils/common/credentials.ts is the right fix in the right place, but adoption stopped at the three handlers. Counted by grep: bstack/constants.ts 19, this file 12, bstack/commands.ts 5, bstack/configUtils.ts 2 — 38 bare literals, and nothing under sdk-utils/bstack/ imports the new module at all.
The drift risk that produced the round-1 convention split is unchanged for those sites.
Suggestion: import USERNAME_PLACEHOLDER / ACCESS_KEY_PLACEHOLDER from the shared module across bstack/** and this file.
Reviewer: stack:code-review
| export const STEP_DELIMITER = "---STEP---"; | ||
|
|
||
| export { | ||
| USERNAME_PLACEHOLDER, |
There was a problem hiding this comment.
[Low] Pass-through re-export shim with a now-false comment
These constants are re-exported from the shared module and are no longer in this file's local scope. It compiles only because nothing in constants.ts references them — but the file header still says it lets other modules "safely read the placeholder constants below at module scope", which is now misleading, and a future edit inside this file that references them will fail to resolve.
Separately, appium-sdk/index.ts does export * from "./constants.js", so the appium barrel now transitively re-exports symbols owned by the sdk-utils tree.
Suggestion: have the ~6 appium consumers (formatter.ts, languages/*.ts) import from ../../sdk-utils/common/credentials.js directly and drop the shim; at minimum fix the comment.
Reviewer: stack:code-review
| export const CREDENTIALS_SUBSTITUTION_NOTE = | ||
| `Replace ${USERNAME_PLACEHOLDER} and ${ACCESS_KEY_PLACEHOLDER} in the steps below ` + | ||
| `with your BrowserStack credentials from https://www.browserstack.com/accounts/profile/details ` + | ||
| `(or export them as BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY). Do not commit real credentials.`; |
There was a problem hiding this comment.
[Low] The "or export" branch conflicts with the literal yml placeholders
This offers "or export them as BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY", but the yml emitters write literal userName: <your_browserstack_username> into browserstack.yml (bstack/configUtils.ts:25-26, bstack/constants.ts:480-481). An agent that takes the export branch still commits a file containing placeholder text — it works only because the SDK's env vars out-rank the yml.
Suggestion: narrow to "…export them instead of editing browserstack.yml", or have the yml step state that the placeholders may be replaced with ${BROWSERSTACK_USERNAME} / ${BROWSERSTACK_ACCESS_KEY}.
Reviewer: stack:code-review
| ["python", ["pytest", "robot", "behave", "lettuce"]], | ||
| ["nodejs", ["jest", "mocha", "cucumberJs", "webdriverio", "nightwatch"]], | ||
| ["ruby", ["cucumberRuby"]], | ||
| ["csharp", ["nunit", "mstest", "xunit", "specflow", "reqnroll"]], |
There was a problem hiding this comment.
[Low] These rows exercise code unreachable in production, and the comment is false
The access-key assertion was added (:72) and this row does pass — getCSharpSDKCommand() genuinely emits both placeholders, so it isn't vacuous. But SUPPORTED_CONFIGURATIONS.appium.csharp is [] (appium-sdk/types.ts:73), so validateSupportforAppAutomate rejects every appium+csharp request before these generators run. The test reaches them only by calling the generators directly.
The framework names are real enum members, but the header comment "Every supported App Automate language/testing-framework combination" (:8) is untrue, and nothing guards COMBOS against drifting from SUPPORTED_CONFIGURATIONS.
Suggestion: derive COMBOS from SUPPORTED_CONFIGURATIONS.appium and keep the csharp rows in a separate, explicitly-labelled "unsupported but must not leak" block — self-maintaining, and honest about reachability.
Reviewer: stack:code-review
| expect(serialized).not.toContain("super-secret-site-password"); | ||
| expect(serialized).not.toContain("site-user"); | ||
| // Safe identifying fields are still returned. | ||
| expect(serialized).toContain("auth-1"); |
There was a problem hiding this comment.
[Low] This assertion cannot fail
toContain("auth-1") is meant to prove the allowlisted payload came through, but the response also carries a separate ID: ${result.data?.id} line (accessibility.ts:292) with the same value. The assertion passes even if safeAuthConfigData returned undefined, so only the get-path assertion is load-bearing.
Suggestion: assert on the allowlist shape instead — e.g. that the payload contains "type" and does not contain usernameSelector — so it fails if the allowlist stops emitting.
Reviewer: stack:code-review
| const steps: RunTestsStep[] = []; | ||
| const authString = getBrowserStackAuth(config); | ||
| const [username, accessKey] = authString.split(":"); | ||
| void config; |
There was a problem hiding this comment.
[Low] void config asymmetry, and breaking exports still undocumented
The explanatory comment is an improvement, but the asymmetry stands: the sibling runPercyWithBrowserstackSDK took the opposite decision and dropped its config param entirely.
More consequentially, the positional-parameter removals on eight exported symbols (getSDKPrefixCommand, getAppSDKPrefixCommand, generateBrowserStackYMLInstructions, generateAppBrowserStackYMLInstructions, getInstructionsForProjectConfiguration, formatEnvCommands, createEnvStep, runPercyWithBrowserstackSDK) plus the ConfigMapping.instructions type change are still documented nowhere. Per CLAUDE.md this package is consumed as a library by the separately-built remote MCP wrapper — and because the removals are positional, a stale caller silently passes username where appPath is now expected instead of failing to compile.
Suggestion: pick one convention for the unused param, and list the export changes in the PR body so the wrapper is updated in lockstep.
Reviewer: stack:code-review
Claude Code PR ReviewPR: #359 • Head: 2e14c0d • Reviewers: stack:code-review Continues the previous review — changes since SummaryRemoves live BrowserStack credentials from generated SDK setup instructions across the Automate/App Automate/Percy flows and locks down the accessibility auth-config tool responses. Commit Both Mediums from the previous round are fixed. Review Table
FindingsResolved since
|
…ential leak)
The success-path logger.info still stringified the whole API response through
redactAuthConfigResponse, which only masked top-level password/username — so a
nested authData.{password,selectors} echo would write the customer's site
password to logs. Switch it to the safeAuthConfigData allowlist ({id, name,
type, url}), matching the tool responses, and delete the now-unused
redactAuthConfigResponse helper.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude Code PR ReviewPR: #359 • Head: 91ccfb0 • Reviewers: fallback inline checklist Continues the previous review — changes since
Summary
Review Table
FindingsResolved since
|
2 participants
No description provided.