fix(plugin-sdk): bound Anthropic Vertex ADC credential read#111120
Conversation
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 9:49 PM ET / July 20, 2026, 01:49 UTC. Summary PR surface: Source +11, Tests +14. Total +25 across 3 files. Reproducibility: yes. The focused regression path writes a real 2 MiB ADC file through GOOGLE_APPLICATION_CREDENTIALS and verifies that current unbounded behavior is replaced by a false preflight result, while a normal file remains detected. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the narrow shared-reader reuse while retaining the established 1 MiB rejection behavior and its normal-size/oversize regression coverage. Do we have a high-confidence way to reproduce the issue? Yes. The focused regression path writes a real 2 MiB ADC file through GOOGLE_APPLICATION_CREDENTIALS and verifies that current unbounded behavior is replaced by a false preflight result, while a normal file remains detected. Is this the best way to solve the issue? Yes. Reusing the existing shared bounded secret-file reader is the narrowest maintainable fix and aligns the plugin-SDK duplicate with the previously merged Anthropic Vertex reader contract. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against 27f05c8993fb. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +11, Tests +14. Total +25 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (6 earlier review cycles)
|
f60cc5f to
640e24f
Compare
|
CI is red on two unrelated flaky timing tests, not on this change. 1.
2.
Green on this change: I couldn't re-run the failed jobs via the API (needs admin rights on the repo). Could you re-run |
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
640e24f to
6a25181
Compare
clawsweeper
Bot
added
rating: 🦞 diamond lobster
canReadAnthropicVertexAdc read the entire GOOGLE_APPLICATION_CREDENTIALS file via unbounded fs.readFileSync solely to check readability (the contents are discarded). The provider-local copy of this logic in extensions/anthropic-vertex/region.ts was already bounded with tryReadSecretFileSync + a 1 MiB limit in openclaw#109260 ("reject oversized credential files in remaining readers"); this standalone plugin-sdk preflight helper is a duplicate that sweep missed. Mirror the region.ts bound: replace the unbounded readFileSync with tryReadSecretFileSync(..., { maxBytes: 1 MiB, rejectHardlinks: false }), wrapped in try/catch (the helper throws FsSafeError on oversize) so an oversized credential file is rejected instead of slurped into memory. Presence-check semantics are unchanged for normal-sized files.
16483c1 to
190f136
Compare
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(plugin-sdk): bound Anthropic Vertex ADC credential read This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Merged via squash.
|
…#111120) * fix(plugin-sdk): bound Anthropic Vertex ADC credential read canReadAnthropicVertexAdc read the entire GOOGLE_APPLICATION_CREDENTIALS file via unbounded fs.readFileSync solely to check readability (the contents are discarded). The provider-local copy of this logic in extensions/anthropic-vertex/region.ts was already bounded with tryReadSecretFileSync + a 1 MiB limit in openclaw#109260 ("reject oversized credential files in remaining readers"); this standalone plugin-sdk preflight helper is a duplicate that sweep missed. Mirror the region.ts bound: replace the unbounded readFileSync with tryReadSecretFileSync(..., { maxBytes: 1 MiB, rejectHardlinks: false }), wrapped in try/catch (the helper throws FsSafeError on oversize) so an oversized credential file is rejected instead of slurped into memory. Presence-check semantics are unchanged for normal-sized files. * refactor(plugin-sdk): remove orphaned vertex auth helper --------- Co-authored-by: Peter Steinberger <[email protected]>
2 participants
What Problem This Solves
The original PR found an unbounded ADC credential read in
src/plugin-sdk/anthropic-vertex-auth-presence.ts.Current-main review showed that this file was no longer a Plugin SDK surface or runtime path: it had no package export, no entrypoint manifest entry, and no production caller. Only its two colocated tests remained. The real Anthropic Vertex plugin already owns the bounded 1 MiB ADC reader in
extensions/anthropic-vertex/region.ts.Why This Change Was Made
The branch now removes the orphaned duplicate helper and its tests instead of adding a second copy of the provider-owned size policy. This is the clean owner-boundary fix: the only live Anthropic Vertex auth path remains bounded, and unreachable Plugin SDK code cannot drift again.
The tooling routing fixture was retargeted to an existing Plugin SDK test so it continues to prove the full Plugin SDK Vitest lane.
User Impact
No user-visible or public SDK behavior changes. The deleted module was not package-exported and had no production caller. Anthropic Vertex discovery continues through the plugin-owned bounded ADC implementation.
Evidence
node scripts/run-vitest.mjs src/scripts/test-projects.test.ts— 85 passed.node scripts/run-vitest.mjs extensions/anthropic-vertex/region.adc.test.ts— 2 passed.node --max-old-space-size=8192 scripts/plugin-sdk-surface-report.mjs --check— 313 SDK entrypoints, 0 forbidden package exports, no surface delta.git diff --checkpassed.Credit: original unbounded-read report and bounded-reader patch by @xydt-juyaohui.