fix(blaze): bind instance storage root - #2471
Conversation
b94e624 to
e8bdf50
Compare
|
@codex review This rewritten one-commit PR binds only the mutable instance storage root, keeps image reads and recovery out of scope, and has passed the exact-tree Linux default, all-feature, strict Clippy, rustdoc, and focused ownership/cleanup matrix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8bdf5091e
ℹ️ 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".
e8bdf50 to
030c3f1
Compare
|
@codex review The two findings on reconstruction error classification and diff-artifact initialization are fixed in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 030c3f1d5b
ℹ️ 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".
030c3f1 to
d9030c9
Compare
|
@codex review The remaining wrong-type reconstruction finding is fixed in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9030c9947
ℹ️ 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".
d9030c9 to
9fb3d91
Compare
There was a problem hiding this comment.
- 实例存储根从规划到准备阶段的身份校验存在空隙,若后续指向不同目录可能绕过生命周期边界约束。
- 生命周期状态根按单一
state_dir解析,未全面覆盖多挂载场景下的实例根与状态根交叉风险。 - 阻塞 acquire 事务“可能残留所有权”路径与 warm 验证的剩余清理契约不完全对齐,易触发不必要隔离/删除。
🤖 Generated by Qoder • View workflow run
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fb3d91ee2
ℹ️ 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".
96f4e65 to
b96a1fe
Compare
|
@codex review Verify that the file provider retains exclusive instance-root ownership, keeps mutable operations descriptor-relative, and never returns backend-visible storage paths through a replaced root. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b96a1fec93
ℹ️ 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".
|
@codex review |
b96a1fe to
1611637
Compare
|
@codex review Bind the mutable instance-storage root to one retained directory object while safely handling opaque Linux mount roots. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16116370d9
ℹ️ 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".
1611637 to
7946157
Compare
|
@codex review Reject resolved storage-root aliases, including dangling symbolic links and bind mounts, before provider-owned paths are created. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
7946157 to
fbee4a9
Compare
|
@codex review Verify that resolved storage-root aliases fail before materialization and that simulated restart tests use the provider's bounded lock-reopen path. |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
|
@codex review |
|
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". |
fbee4a9 to
b69328d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b69328d68f
ℹ️ 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".
d8e3df8 to
fc28fb8
Compare
|
@codex review Verify that the exact head atomically publishes retained storage directories and keeps cancelled acquisition under lock without changing safe-layout behavior. |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
|
@codex review Please review the exact head's atomic publication of retained storage directories, cancellation supervision, and compatibility with the current main branch. |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
Retain an exclusive descriptor owner for storage.instances_dir from startup planning through the file-provider lifetime. Descriptor-relative operations prevent path replacement from redirecting mutable slot work without removing the public StorageSlot paths used by backends. Keep storage.images_dir reads path-based. Leave slot inventory, startup recovery, and warm storage to follow-up work. Fixes: 8cf1df2 ("feat(blaze): implement FileStorageProvider with unit tests") Signed-off-by: Weisson <Weisson@linux.alibaba.com>
Publish missing storage components and sandbox slots through private, descriptor-retained staging directories and no-replace renames. Reject path chains whose permissions allow untrusted replacement while keeping first-start creation. Keep storage acquisition under detached per-sandbox supervision so request cancellation cannot let destroy overtake filesystem work. Carry cleanup disposition explicitly and suppress stable-name cleanup when identity cannot be proven. A process crash before staging publication can leave an empty hidden directory. Root and same-effective-user path mutation remains an administrative trust boundary. Signed-off-by: Weisson <Weisson@linux.alibaba.com>
fc28fb8 to
03616fc
Compare
|
@codex review Please review exact head |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
|
@casparant All 11 review threads are now resolved. The rewritten exact head |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
Why
The file storage provider must keep mutable sandbox storage bound to the
directory Blaze validated and opened at startup. Re-resolving
storage.instances_dir, creating a missing directory under its final namebefore retaining it, or releasing the sandbox operation lock while blocking
storage acquisition was still running could redirect work or make cleanup act
on a replacement directory.
What changed
storage.instances_dirbefore creating daemon-ownedpaths. Reject unsupported path components, overlap with daemon state, and
symbolic-link or bind-mount aliases with
storage.images_dir.component against its opened descriptor.
or the daemon's effective user and to reject group or other write access. A
shared writable existing ancestor is accepted only when it is sticky and its
next existing component has a trusted owner.
names. Retain their descriptors and filesystem identities, publish them with
an atomic no-replace rename, synchronize the parent, and verify the published
names before use.
name changes identity, acquisition fails without adopting or deleting the
replacement.
cancellation. Destroy waits until acquisition finishes or establishes that
cleanup by sandbox identifier is unsafe.
retained instances root. Revalidate ordinary backend-visible paths before
returning them.
unexpected entry types, mount changes, and object replacement.
replacement, cancellation, provider failure, rollback, release, and relative
path resolution.
compatibility boundary and operator recovery steps.
Base-image reads through
storage.images_dirremain path-based. Backendsstill receive ordinary host paths. Replacement after provider handoff but
before a backend opens the path remains tracked by #2495 and is not claimed by
this change.
Related issue
Closes #2484
Refs #2254
Follow-up backend path handoff: #2495
User / Agent impact
The storage configuration fields, HTTP APIs, successful sandbox lifecycle, and
on-disk slot format are unchanged. Existing safe layouts require no migration,
and first-start creation remains supported when the parent path satisfies the
ownership and permission checks.
Blaze now rejects unsafe path aliases, shared writable publication parents, a
root owned by another daemon, and names already occupied at atomic
publication. If the configured instances path is renamed or replaced while
Blaze is running, new allocation and reconstruction fail instead of using the
replacement. Work that already retained the original root continues against
that root.
Canceling a create request no longer permits destroy to overtake unfinished
storage acquisition. If Blaze cannot prove that a published slot name still
identifies its directory, automatic cleanup by sandbox identifier is disabled
for the current daemon process and the sandbox requires inspection.
Privileged processes and processes running with the daemon's effective user
remain within the host administration boundary. Cleanup suppression after a
detected replacement is not persisted across daemon restart, so stop Blaze and
inspect or restore the storage path before restarting after an administrative
path change.
A process exit after private staging creation but before publication or cleanup
can leave an empty
.blaze-dir-<uuid>.tmpdirectory. Blaze does not adopt ordelete such restart residue based only on its name.
Risk and compatibility
The visible change is explicit rejection and recovery handling for unsafe
filesystem conditions. Normal configuration, APIs, successful operation, and
stored sandbox data remain compatible.
Commit structure
6b4a8a50030a302bd163bff6a841e4a9d586f88b— bind the validatedinstances root to a retained descriptor owner and move provider lifecycle
operations beneath it.
03616fc0a18d06ec5d170df344e2dc2465e1ec10— add private no-replacepublication, identity and durability checks, cancellation-safe acquisition,
and fail-closed cleanup.
The previous validation-only correction commit was folded into the commit that
introduced each affected invariant. The final source tree is unchanged from
the reviewed current-main candidate.
Validation
Revision identity:
ba20d94d19a73e5b781cab0c687d4034ad0d0dae, treeb69ad4b5dd999e7244b1cea058e62232e7d1407a;6b4a8a50030a302bd163bff6a841e4a9d586f88b, tree31fbf0d9889980e56372cbdd5f20719706d59e3a;03616fc0a18d06ec5d170df344e2dc2465e1ec10, tree32d16adabaea187c620040136551a12dcb153978;bf65d19d89a1d6216117d6ce5e1f09eae0c9a15c, tree32d16adabaea187c620040136551a12dcb153978, with ordered parentsba20d94d19a73e5b781cab0c687d4034ad0d0daeand03616fc0a18d06ec5d170df344e2dc2465e1ec10.Each public object was independently reconstructed from GitHub's public commit,
tree, blob, and source-archive interfaces, then validated on native Linux
x86_64 with Rust and Cargo 1.88.0, locked offline dependencies, a fresh Cargo
home, and an empty target directory for every stage.
For the first commit:
Clippy, serial tests, and strict rustdoc passed;
blaze-core53 andblazed280;blaze-core53 andblazed301;For the exact head and GitHub merge ref, each in a separate fresh environment:
blaze-core53 andblazed288;blaze-core53 andblazed314;passed;
The source tree remained unchanged in all three runs. Final source-archive
SHA-256 values were:
5288dacf0ffa9624550219d3dd9e3eb8ba012c8827cd80c0d44ecf846b46a906;465573b56da73762bbe8c8793bc7e50ce02acabfa26c47917c6285505c664c3b;15288ea8eff25330fd6c0975fce2323f1df56ecd82c1cf1c7a095f7f7caef4fd.Hosted checks for the rewritten exact head passed:
GitHub's public commit status surface does not currently expose an independent
CLA result for this rewritten head, so no CLA result is claimed here. The
exact-head Codex review
reviewed
03616fc0a1and reported no major issues.Documentation and rollback
The English and Chinese README and existing user guide describe accepted and
rejected layouts, pathname replacement, request cancellation, the
administration boundary, restart limitations, and operator recovery.
To roll back, stop every daemon using the instances root before restoring the
previous binary. No data migration is required, but the earlier path-based
failure behavior returns after rollback.
All review threads are resolved. No unresolved inline finding remains for the
rewritten exact head.