Skip to content

Fix double-free of path in S3 static methods on error paths - #28495

Closed
robobun wants to merge 2 commits into
mainfrom
farm/be997ae5/fix-s3-path-double-free
Closed

Fix double-free of path in S3 static methods on error paths#28495
robobun wants to merge 2 commits into
mainfrom
farm/be997ae5/fix-s3-path-double-free

Fix double-free of path in S3 static methods on error paths

ad68949
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 6, 2026 in 26m 52s

Code review found 2 potential issues

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/bun/s3/s3-presign-path-deinit.test.ts:9-20 Tests are non-hermetic: empty-string credentials do not override ambient AWS env vars

Annotations

Check warning on line 20 in test/js/bun/s3/s3-presign-path-deinit.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Tests are non-hermetic: empty-string credentials do not override ambient AWS env vars

Both tests in this file are non-hermetic on machines with `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY` (or `S3_*`) exported. The static test on L19 passes no options at all, and the instance test's `{ accessKeyId: "", secretAccessKey: "" }` does **not** override env credentials — `getCredentialsWithOptions` reads these via `getTruthyComptime`, which treats empty strings as absent — so the comment on L9-10 is incorrect. CI is unaffected (it doesn't set these vars), but for local hermeticity the te