fix(sdk): keep restored scan artifacts within their output directory - #468
fix(sdk): keep restored scan artifacts within their output directory#468mldangelo-oai wants to merge 5 commits into
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the current head, |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review commit |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the current head |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8fc845591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const path = join(scanDir, ...parts); | ||
| const metadata = await lstat(path); | ||
| return { path: join(scanDir, ...parts), parents }; |
There was a problem hiding this comment.
If a background command started during the post-scan phase replaces a validated parent directory with a symlink after this check returns, the subsequent writeFile(temporary, ...) and rename(temporary, path) follow that replacement and can overwrite a matching file outside the scan directory. The helper returns only a pathname, so none of the captured parent identities are bound to or revalidated around the actual restoration write; keep the write and rename tied to the checked directory identity so a concurrent swap fails safely.
AGENTS.md reference: sdk/typescript/AGENTS.md:L19-L20
Useful? React with 👍 / 👎.
| const deadline = | ||
| platform === "win32" | ||
| ? (readRetry.deadline ??= | ||
| Date.now() + INCOMPLETE_CREDENTIAL_LOCK_MILLISECONDS) | ||
| : undefined; |
There was a problem hiding this comment.
On Windows, if one lock remains unreadable for most of this deadline and is then released and quickly replaced before the waiter observes an absent directory, readRetry.deadline is carried over to the new lock because the lstat identity is not tracked. A transient EPERM or EBUSY on the replacement can therefore exceed the old lock's deadline immediately and fail an otherwise healthy scan; associate the retry state with the lock's device/inode and start a fresh deadline when that identity changes.
AGENTS.md reference: sdk/typescript/AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
1 participant
Summary
Keep completed scan artifact restoration within the selected scan output directory.
Changes
Testing
Current head
e8fc8455:pnpm run test --randomize --seed 12345: 1,311 passed, 11 skipped, 0 failed.pnpm run test: 1,311 passed, 11 skipped, 0 failed.pnpm run types,pnpm run format,pnpm run build, andgit diff --check: passed.3368d07e.Risk and rollout
Normal completed-artifact restoration remains unchanged. Public contract readers keep their existing path canonicalization, including supported output directories beneath directory links. Restoration uses the same non-symlink artifact-parent requirement as completed-contract reads and stops if the approved canonical output root has changed. The Windows retry does not reclaim an unreadable lock or hide persistent errors. No public SDK export, dependency, or runtime setting is added.
Public disclosure review
Newly authored material was reviewed for public disclosure. Existing automated review comments contain access-restricted report references; maintainer cleanup remains outstanding.