feat(workspace): add package manager torture lab - #35
Conversation
Convert the schema u32 mode through the host mode_t before calling nix so Darwin and Linux compile the same archive path. Verified with cargo test --all-targets on macOS. Co-authored-by: Codex <codex@openai.com>
Replace retry sleeps with the daemon readiness stream, keep stderr drained, and scope ELF assertions to Linux. Verified by the focused git fixture and the 63-test macOS baseline. Co-authored-by: Codex <codex@openai.com>
Keep configured providers as solver preferences, derive LMDB bindings from the actual solution, and stop rebuild from requesting preferences as hard roots. The regression proves conflict backtracking from musl to glibc. Co-authored-by: Codex <codex@openai.com>
Validate every payload header before the first write, reject duplicate canonical paths and unsafe symlink targets, and expose explicit rename/write fault boundaries for the Torture Lab. Co-authored-by: Codex <codex@openai.com>
Add all-or-nothing batch publication, pre-write/pre-commit fault injection, full ownership readback, and nonblocking host-lock probes for deterministic concurrency tests. Co-authored-by: Codex <codex@openai.com>
Preflight archive identity and ownership before journaling, checkpoint completed trigger groups, use collision-resistant operation IDs, add abrupt-failure hooks, and provide count/verify consistency commands. Co-authored-by: Codex <codex@openai.com>
Validate repository names, aliases, signing-key paths and target roots before they participate in cache or rootfs path construction. Regression fixtures cover each traversal source. Co-authored-by: Codex <codex@openai.com>
Add signed package fixtures, per-test rootfs and LMDB isolation, a reference-model random state machine, crash/retry matrices, path attacks, real process concurrency, state audits and repeatable benchmarks. Co-authored-by: Codex <codex@openai.com>
Run the quick hermetic gate in normal CI and add manual/nightly jobs for long fixed-seed state machines and 10,000-package release benchmarks. Co-authored-by: Codex <codex@openai.com>
Document invariants, failure reproduction, fault-point extension, CI tiers, measured benchmarks, verified fixes, platform limits, SCLinux pin status and remaining release risks. Co-authored-by: Codex <codex@openai.com>
Apply rustfmt to the duplicate-canonical-path assertion before the final release gates. Co-authored-by: Codex <codex@openai.com>
Record every build, lint, test, benchmark and SCLinux integration gate, including the verified orphaned pin blocker and the incomplete local Linux image pull. Co-authored-by: Codex <codex@openai.com>
Keep the 9,568-line production count visible as a release risk rather than deleting reliability checks to satisfy the historical 9,000-line target. Co-authored-by: Codex <codex@openai.com>
Expand the random model with upgrades, rollback attempts, dependency replacement, channel switching and prefix shrinking; add file-type, permission, long-path, hard-link, map-full and repeated-recovery coverage; anchor operation locks with openat(O_NOFOLLOW), add bounded waits, and make removal idempotent when a managed subtree is already absent. Co-authored-by: Codex <codex@openai.com>
Record the expanded matrices, final 100/1000/10000 measurements, 1000-step random run, macOS gates, ARM64 Linux cross-build and Debian execution, SCLinux pin blocker, approval-gated remote issue, and updated source budget. Co-authored-by: Codex <codex@openai.com>
Gate archive/database fault and benchmark seams behind explicit torture features, narrow HostLock to ordinary and bounded acquisition, share declaration parsing between preflight and recovery, remove the speculative count command, and stop duplicating quick/benchmark work in normal CI. Co-authored-by: Codex <codex@openai.com>
Document feature-gated torture hooks, narrowed lock APIs, shared declaration parsing, deferred SCLinux count compatibility, deduplicated CI, informational source size, and final macOS/Linux verification counts. Co-authored-by: Codex <codex@openai.com>
Gate CLI crash markers behind the same explicit torture feature as archive/database hooks, forward the feature through the Sage crate, keep ordinary parent lock directories at 0755 while retaining a private 0700 Sage directory, and cover the resulting permissions. Co-authored-by: Codex <codex@openai.com>
State that ordinary Sage builds neither expose fault enums nor honor crash markers, and that sage-tests enables one forwarded torture feature across the CLI, archive, and database crates. Co-authored-by: Codex <codex@openai.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 12cc81bc05
ℹ️ 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".
Compute final ownership for every changed package, order releasing owners before new claimants, and reject cyclic swaps before journaling. Replace timed-lock busy spinning with deadline-bounded sleeps and retain deterministic timeout coverage. Co-authored-by: Codex <codex@openai.com>
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2458b56a8
ℹ️ 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".
| fn valid_identifier(value: &str) -> bool { | ||
| !value.is_empty() | ||
| && value | ||
| .bytes() | ||
| .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-')) |
There was a problem hiding this comment.
Reject . and .. explicitly rather than accepting every string composed of punctuation allowed here. For example, a configured channel and alias both named .. pass this predicate, after which sync_channels builds the destination with cache.join(channel_name).join(alias), escaping the per-channel cache tree and potentially overwriting another index or Sage cache file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 191f969 修复：valid_identifier 现在显式拒绝 . 和 ..；回归覆盖 channel、subchannel 名和 alias 三个入口各自的两个 dot-segment。针对性测试、66 项主集成测试和完整 CI 均通过。
| let selected = solution.iter().find_map(|(key, version)| { | ||
| universe | ||
| .release(key, version) | ||
| .is_some_and(|package| package.provides.contains(&symbol)) | ||
| .then(|| key.clone()) |
There was a problem hiding this comment.
Derive this binding from the selected virtual proxy rather than the first concrete package in solution order. When two selected packages provide the same symbol—for example, one is an explicit root while another satisfies a constrained virtual dependency—this find_map can choose the former regardless of version constraints or provider preference, causing LMDB to record a different system provider from the one PubGrub actually used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 191f969 修复：SageSolver 在过滤私有 __sage proxy 前，从已选 proxy release 的唯一 concrete dependency 提取实际 Provider；ReconcilePlan 直接使用该结果，不再按 solution 顺序猜测。新增“显式旧 Provider root + 受版本约束的实际 Provider”回归，针对性测试、全量测试和 CI 均通过。
|
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. |
Reject dot-only channel path segments and persist provider bindings from PubGrub's selected virtual proxy. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 191f969147
ℹ️ 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".
| let archive = obtain_release_archive(&engine, &package_cache, source).await?; | ||
| let inspection = sage_archive::inspect_package(&archive)?; |
There was a problem hiding this comment.
When an indexed package contains a structurally invalid payload, such as a hard link, missing indexed entry, or duplicate payload path, inspect_package succeeds because it stops at the first data/ entry. The journal is then written before extract_package_internal performs validate_payload, so extraction fails with no publication but leaves a pending journal that every later mutating command retries and fails on, including the sync needed to obtain a corrected index. Run full payload validation during this preflight, before write_journal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：安装 preflight 在创建 journal 前调用公开的完整 payload 无写入验证。新增恶意 hardlink 包回归，确认安装失败后 pending journals 和 package state 均为空，随后健康包仍可正常安装；全量测试和两条 CI 均通过。
| } else if entry.header().entry_type().is_file() { | ||
| if entry.size() != record.size { | ||
| return Err(ArchiveError::InvalidMetadata(format!( | ||
| "size mismatch for {}", | ||
| relative.display() | ||
| ))); | ||
| } |
There was a problem hiding this comment.
When a later regular entry has the declared size but contents that disagree with files.idx, this scan accepts it because it checks only entry.size(). Extraction can therefore atomically publish earlier entries before detecting the bad checksum while writing the later entry, leaving a partially upgraded root and a journal that cannot complete on retry. Consume and hash each regular entry during the no-write validation pass so all checksum failures occur before the first rename.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：payload 预检对每个 regular entry 复用流式 verify_reader，提前校验 size 与 SHA-256。新增“首条有效、后置同长度坏哈希”回归，确认失败时首条文件也未发布；全量测试和两条 CI 均通过。
| if !seen.insert(relative.to_path_buf()) { | ||
| return Err(ArchiveError::InvalidMetadata(format!( | ||
| "duplicate payload {}", | ||
| relative.display() | ||
| ))); |
There was a problem hiding this comment.
When the index contains both a file or symlink path and one of its descendants, such as opt/app and opt/app/bin/tool, this validation only detects exact duplicates and accepts both. Extraction then publishes whichever entry appears first and fails when the other entry tries to treat the same path as the opposite filesystem type, leaving a partial package state despite the promised pre-write validation. Reject any indexed non-directory path that is an ancestor of another indexed path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：完整 payload 验证在任何写入前检查所有 indexed path 的祖先，拒绝非目录索引项同时作为另一项的祖先。新增 opt/app 与 opt/app/bin/tool 回归，确认两者均未落盘；全量测试和两条 CI 均通过。
Validate archive structure and regular-file hashes before creating install journals, and reject indexed ancestor collisions before any filesystem publication. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd7c5a6cb1
ℹ️ 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".
| if relative.as_os_str().is_empty() || entry.header().entry_type().is_dir() { | ||
| continue; |
There was a problem hiding this comment.
When an archive includes an extra directory such as data/var/unowned/, this branch skips it during preflight, but extraction later creates it with ensure_directory even though it is absent from files.idx and package ownership. A successful install therefore leaves unmanaged filesystem debris that removal cannot clean up; a directory colliding with an indexed file can instead make extraction fail after earlier files were published. The fresh case not covered by the indexed-ancestor fix is that directory headers never enter either collision or membership validation. Validate directories against the implied ancestor set of indexed paths before allowing them.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 72b3e97 修复：payload validator 从 indexed leaf 构造必要祖先目录集合，目录 header 只有属于该集合才允许；额外空目录及与 indexed leaf 同路径的目录均在首次写入前拒绝。新增两类零落盘回归，完整测试和两条 CI 均通过。
| 0o755 | ||
| }), | ||
| ) { | ||
| Ok(()) | Err(Errno::EEXIST) => {} |
There was a problem hiding this comment.
When the final lock directory already exists with permissions allowing another user to rename entries, accepting EEXIST without checking its owner or mode leaves the host lock replaceable: that user can unlink operation.lock while one process holds its inode, after which another Sage process creates and locks a different inode and both writers proceed concurrently. This can occur with a pre-created or misconfigured /run/sage; verify and repair the directory's ownership and permissions before opening the lock file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 72b3e97 修复：最终专属锁目录经 anchored openat(O_NOFOLLOW) 打开后，在创建 operation.lock 前用 fstat/fchown/fchmod 修复到 effective owner 与 0700，并二次验证；非 root 遇到外来 owner 时安全失败。新增预建 0777 目录回归，完整测试和两条 CI 均通过。
Reject payload directory headers outside indexed ancestor paths and repair the anchored lock directory owner and mode before opening the lock file. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72b3e9778c
ℹ️ 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".
| - Production Rust source size is informational for this release; correctness | ||
| gates are not weakened to meet the historical 9,000-line target. |
There was a problem hiding this comment.
Do not demote the source-size limit to an informational metric: this commit adds 3,215 net Rust lines and leaves the tracked workspace at 14,713 lines, despite the repository's strict whole-workspace maximum of 9,000. The newly added torture code is part of the workspace, so it must be reduced or consolidated rather than documenting the mandatory gate as waived.
AGENTS.md reference: AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
Remove duplicate reports, speculative CLI surfaces, redundant fault APIs and repeated test permutations while preserving the core model, recovery, archive, ownership and concurrency gates. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 470698dd1f
ℹ️ 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".
| if let Some(previous) = bindings.insert(symbol.clone(), key.clone()) { | ||
| if previous != key { | ||
| return Err(SolverError::Internal(format!( | ||
| "virtual symbol {symbol} selected both {previous} and {key}" | ||
| ))); |
There was a problem hiding this comment.
When two desired roots impose non-overlapping constraints on the same virtual symbol and coinstallable packages satisfy each constraint, the initial apply_packages call in rebuild_system uses resolve() and publishes both providers, but this later binding pass detects the two keys and returns an internal error. The rebuild therefore fails only after changing the filesystem and LMDB, before pruning, service rendering, and rebuild triggers; perform this binding validation during the initial solve or otherwise before publishing packages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1c72c7e 修复：rebuild 现在先加载 available/installed 并执行带 Provider binding 的完整 ReconcilePlan 求解，只有成功后才进入 apply_packages，因此同一接口出现不兼容 Provider 选择时不会先修改文件系统或 LMDB。新增双 root 非重叠 virtual/libc 约束回归；全量测试和两条 CI 均通过。
| let provider_bindings = preferences | ||
| .into_iter() | ||
| .map(|(symbol, key)| (symbol.strip_prefix("virtual/").unwrap_or(&symbol).into(), key)) | ||
| .into_keys() | ||
| .filter_map(|symbol| { | ||
| let selected = selected_providers.get(&symbol)?.clone(); |
There was a problem hiding this comment.
When a configured interface is no longer referenced by any desired package, selected_providers has no entry and this filter_map silently omits it. rebuild_system only calls set_system_provider for returned entries and has no deletion pass, so a previous binding remains in LMDB even when reconciliation prunes its provider package; emit an explicit removal or replace the complete binding set during reconciliation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1c72c7e 修复：DB API 改为单事务 replace_system_providers，先清空旧集合再写入本次完整 binding；不再被选择的接口会被删除。新增写入 libc binding 后以空集合替换的回归；全量测试和两条 CI 均通过。
Compute the complete reconciliation plan before package mutation and replace persisted provider bindings atomically so incompatible or removed interfaces cannot leave partial state. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c72c7e97f
ℹ️ 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".
| let releases = final_paths | ||
| .get(&owner) | ||
| .is_some_and(|paths| !paths.contains(path)); |
There was a problem hiding this comment.
When package A releases a modified configuration path such as /etc/app.conf and package B claims it in the same upgrade, this handoff ordering publishes A first: A's obsolete-file cleanup preserves the locally modified file, but B then extracts with only B's previous_packages hashes (normally none for a newly claimed path), so write_verified renames B's file over the preserved configuration. Transfer the releasing owner's config hash to the claimant or otherwise preserve the live file across the handoff to avoid irreversible user configuration loss.
AGENTS.md reference: AGENTS.md:L76-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 42ddce5 修复：journal 中已有的 previous_packages 现在先构造 physical path → config hash 映射；同事务接管该路径的新包会继承释放方哈希。若 live config 已被用户修改，接管包保留原文件并将新内容写为 .sage-new。现有 ownership handoff 回归已改为配置文件场景，同时保留循环 handoff 原子拒绝；全量测试和两条 CI 均通过。
Carry the releasing package config hash to its same-transaction claimant so locally modified files survive ownership changes and new content is written as .sage-new. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ddce5887
ℹ️ 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".
| let plan = | ||
| sage_sys::ReconcilePlan::compute(&config, &installed, &available.universe, no_prune)?; | ||
| let desired: Vec<_> = config.packages.iter().cloned().collect(); | ||
| apply_packages(root, &desired, Some("system"), false, false, dry_run).await?; |
There was a problem hiding this comment.
When switching from an installed provider to a conflicting replacement, this call discards plan.install and runs a second solve in apply_packages, where every installed package is made a root. For example, if installed glibc conflicts with preferred musl, ReconcilePlan::compute selects musl and schedules glibc for removal, but the second solve must retain glibc and backtracks to it, installs no musl, and then lines 1146–1152 remove glibc anyway, leaving the virtual dependency unsatisfied and persisting a binding to an uninstalled provider. Fresh evidence after the earlier provider-preflight fix is this divergence between the validated plan and the separate publication solve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 15403b1 修复：rebuild 不再调用 apply_packages 二次求解，而是按依赖顺序直接发布已由 ReconcilePlan 验证的 plan.install 精确版本集合，然后执行同一 plan 的 remove 与 binding 替换。这样 Provider 切换不会在验证后分叉；现有 Provider 切换、冲突 binding 和完整 binding 回归均通过，两条 CI 通过。
| path.display() | ||
| ))); | ||
| } | ||
| file.set_permissions(std::fs::Permissions::from_mode(0o600))?; |
There was a problem hiding this comment.
When the lock namespace was writable before Sage starts, an attacker can pre-create operation.lock as a hard link to an attacker-owned file outside the namespace. O_NOFOLLOW and is_file() both accept that inode, and this line changes the external file's permissions while the attacker retains access through the other link and can later hold its flock to block all Sage operations. The directory-hardening fix does not inspect the file's owner or link count; reject foreign or multiply linked existing files before chmod and locking.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 15403b1 修复：打开 operation.lock 后、chmod/flock 前检查 regular file、effective owner 及 nlink == 1；foreign 或 multiply linked inode 立即拒绝。现有 lock namespace 回归新增外部 hard-link fixture，并保留 symlink 与 permissive-directory 覆盖；全量测试和两条 CI 均通过。
Publish the prevalidated reconciliation solution directly and reject foreign or multiply linked lock inodes before chmod and locking. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15403b1cfd
ℹ️ 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".
| no_prune: bool, | ||
| ) -> Result<Self, SysError> { | ||
| let mut roots = config.package_keys("main/system")?; | ||
| let roots = config.package_keys("main/system")?; |
There was a problem hiding this comment.
When a [providers] entry is not also listed in packages and no desired package currently depends on its virtual symbol, removing the configured package from roots leaves no proxy selection for that interface. The plan then schedules an installed provider for pruning, omits its binding through the later filter_map, and can fail while rendering services after already removing the configured init implementation. Keep configured provider packages in the desired solution while deriving bindings from actual proxy selections where available.
AGENTS.md reference: AGENTS.md:L16-L20
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：先用 soft preference 求解真实 virtual proxy；仅对没有被任何 desired dependency 引用的配置接口，把其显式 Provider 加入 roots 后重算，并验证该包实际 provides 对应 symbol。这样 init 等独立配置 Provider 会保留，同时 libc 等活跃接口仍可冲突回溯。新增未引用 virtual/init 的 systemd 保留与 binding 回归；CI 通过。
| } | ||
| if !dry_run && !changes.is_empty() { | ||
| let database = sage_db::SageDatabase::open(&db_path)?; | ||
| publish_packages( |
There was a problem hiding this comment.
When rebuilding from an installed provider such as glibc to musl while an installed package depends on virtual/libc, this newly added publication installs musl first, but the subsequent remove_packages call still treats removed.provides.contains("virtual/libc") as proof that glibc is uniquely required and does not check the newly installed provider. The rebuild therefore fails after publishing musl but before removing glibc, persisting bindings, or rendering services. Fresh evidence after the exact-plan fix is that publication now succeeds before this unchanged dependency guard rejects the planned removal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：remove dependency guard 在判断旧 Provider 必需前，会检查本次不删除的已安装包是否以相同 channel/slot/version constraint 提供同一 concrete/virtual dependency；已安装替代 Provider 存在时允许按 plan 删除旧 Provider。完整 Provider 切换与全量门禁通过。
| if normal_index == normal_components { | ||
| harden_lock_directory(&next, path)?; | ||
| } |
There was a problem hiding this comment.
When an ancestor of the private lock directory, such as a misconfigured writable /run, can be renamed by another user, hardening only the final /run/sage descriptor does not make the namespace replace-safe: the attacker can rename that directory after this check, create a replacement, and cause the next Sage process to lock a different inode. Fresh evidence after the directory-hardening change is that every intermediate directory is opened but none is checked for ownership or non-replaceability; reject unsafe ancestors or anchor the lock beneath a trusted root.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：除最终私有目录外，openat walk 现在还在进入 sage 前验证直接祖先的 effective owner，且拒绝 group/other writable mode；不可信 /run 在创建私有 namespace 前失败。现有 lock 回归新增预建 0777 run/ fixture，并保留 symlink、hard-link 与 final-directory 修复覆盖；CI 通过。
Keep unused configured providers without hard-binding active proxies, recognize installed replacement providers during pruning, and reject writable lock ancestors. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22706cb9cb
ℹ️ 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".
| } | ||
| if !dry_run && !changes.is_empty() { | ||
| let database = sage_db::SageDatabase::open(&db_path)?; | ||
| publish_packages( |
There was a problem hiding this comment.
When sage rebuild --no-prune introduces a package that conflicts with an unconfigured installed package, ReconcilePlan::compute does not keep that installed package as a solver root and suppresses its removal because pruning is disabled. Publishing this plan therefore installs both packages successfully when their files do not overlap, leaving a system whose installed package set violates declared conflicts. Include every package retained by --no-prune in the validated solution before publication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 4b5f4bc 修复：ReconcilePlan 在 no_prune 时把全部已安装 package key 加入 roots 后再求解，因此新 desired 包与任何保留包冲突都会在发布前无解。该场景并入现有 dependency-closed reconciliation 测试；全量门禁和两条 CI 通过。
| let lock_root = std::fs::canonicalize(&cli.root) | ||
| .with_context(|| format!("cannot resolve target root {}", cli.root.display()))?; | ||
| let lock_path = under_root(&lock_root, Path::new("/run/sage/operation.lock")); |
There was a problem hiding this comment.
When --root is a symlink whose directory can be modified concurrently, only the lock path uses its canonical destination; all subsequent operations still use cli.root. An attacker or orchestrator can retarget the symlink after lock acquisition, causing Sage to mutate a different sysroot while holding the first root's lock, so another process can mutate that second root concurrently. Use the resolved root for the entire operation, not just for constructing the lock path.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 4b5f4bc 修复：execute 现在将 cli.root 本身替换为 canonicalized path，lock path 与后续 sync/install/remove/rebuild/query 全部使用同一个已解析 root，不再继续引用可被重定向的原始 symlink。全量门禁和两条 CI 通过。
Include every retained installed package in no-prune solving and use the canonicalized target root for both locking and all subsequent operations. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5f4bc480
ℹ️ 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".
| &database, | ||
| &available, | ||
| &config.system.architecture, | ||
| &changes, |
There was a problem hiding this comment.
When reconciliation replaces an installed package with one that owns any of the same paths, only plan.install is passed into this publication transaction, so preflight treats the package in the later plan.remove set as a permanent owner and rejects the replacement. Fresh evidence after the earlier handoff fix is that rebuild now processes removals only after this call; provider or conflict replacements with overlapping files therefore still cannot converge. Publish installs and removals as one handoff-aware transaction, including configuration preservation and recovery state.
AGENTS.md reference: AGENTS.md:L76-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：rebuild 将 plan.remove 快照与 plan.install 一起写入现有 Install journal。恢复先释放 retired package 的 DB ownership、发布 replacement，再清理无人持有路径；previous config hashes 与旧 trigger 文档同 journal 保留，支持配置 handoff、PostRemove/PostChange 和崩溃重放。重叠 ownership 不再被视为永久冲突；完整门禁和 CI 通过。
| [dependencies] | ||
| sage = { workspace = true } | ||
| anyhow = { workspace = true } | ||
| sage = { workspace = true, features = ["torture"] } |
There was a problem hiding this comment.
With the documented root-level cargo build --release command, sage-tests is one of the workspace's default members, so this dependency activates and unifies sage/torture while the production sage binary is built. Consequently the release binary includes the /run/sage/crash-point path that can deliberately abort an in-progress package operation. Activate this feature only in dedicated torture invocations or exclude the test crate from the default release workspace build.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：workspace default-members 仅包含生产 crates/sage，CI 改为显式 --workspace 测试/Clippy。已执行默认 cargo build --release 并扫描 target/release/sage，确认不含 crash-point、abort: 或 injected-crash 字符串；CI 通过。
| roots.sort(); | ||
| roots.dedup(); | ||
| if no_prune { | ||
| roots.extend(installed.iter().map(|package| package.key.clone())); |
There was a problem hiding this comment.
When an installed package or its exact version has disappeared from the synchronized repository, adding its key as a solver root does not actually retain that installed release: SageProvider contains only repository releases, and the lock is honored only when its version remains available. Thus rebuild --no-prune either upgrades an undeclared retained package to another repository version or fails with no solution when the key is gone, despite no pruning being requested. Add installed releases as exact solver candidates when validating the retained set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：--no-prune 求解前复制候选 universe，并为仓库缺失的 installed key/version 注入由其持久化 dependencies/provides/conflicts 构造的精确 release；锁定版本因此可继续作为保留 root。现有 reconciliation 测试新增缺失 old release 的 no-prune 成功回归；CI 通过。
Journal planned removals with replacement installs, isolate torture features from default release builds, and retain unavailable installed releases during no-prune solving. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d2f07084
ℹ️ 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".
| let retired = installed | ||
| .iter() | ||
| .filter(|package| plan.remove.contains(&package.key)) | ||
| .cloned() |
There was a problem hiding this comment.
When an installed package outside main/system depends on a package in main/system—for example, a retained main/runtime package with main/system/libc or virtual/libc constraints—ReconcilePlan::compute does not include that retained package as a root during normal pruning, yet this new retirement path bypasses remove_packages' dependency guard and removes every key in plan.remove. A successful rebuild can therefore leave the retained package with an unsatisfied dependency; include all packages that will remain installed in the reconciliation solve, or validate their dependencies before retiring the plan.
AGENTS.md reference: AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1b05a59 修复：ReconcilePlan 现在把所有不会被本次 plan 删除的 cross-channel installed 包加入 roots；若其 exact key/version 已从仓库消失，也会由持久化 dependencies/provides/conflicts 注入临时候选。新增 main/runtime 包依赖 main/system/lib 且 runtime release 不在仓库的回归，确认 plan 保留 runtime 并安装 system 依赖；完整 workspace 测试、quick、固定 seed 随机序列和两条 CI 均通过。
Include every retained non-system package and its persisted exact release in reconciliation so pruning cannot break cross-channel dependencies. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b05a594c7
ℹ️ 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".
| if !retained.is_empty() { | ||
| roots.extend(retained.iter().map(|package| package.key.clone())); |
There was a problem hiding this comment.
When --no-prune retains an installed package whose current version conflicts with a newly desired package, adding only its key as an unconstrained root allows the solver to select another repository version and include an unexpected upgrade in plan.install. Fresh evidence after the unavailable-release fix is that SageSolver::choose_version treats with_locked entries only as preferences and falls back to another matching version during backtracking; for example, retained old v1 conflicting with app will silently become v2 if v2 is available and compatible. Express retained packages as exact-version roots so --no-prune either preserves their installed releases or rejects the reconciliation.
Useful? React with 👍 / 👎.
1 participant
Summary
sage-tests及 fault seam 不属于默认 release build；默认cargo build --release仅构建生产 Sage。docs/TORTURE_LAB.md，nightly/manual workflow 只运行固定 seed 长随机序列。Validation
cargo fmt --all -- --checkcargo build --release，并确认生产二进制不含 torture fault markercargo check -p sage --no-default-featurescargo clippy --workspace --all-targets -- -D warningscargo test --workspace --all-targets：67 项主测试、15 项 Torture Lab 测试通过1517166630的 200 步随机序列通过，共记录 331 个步骤Rust baselineCI 通过Scope reduction
+2,687 / -200；相比瘦身前+3,966 / -141，减少 1,279 行新增内容。sage verify、--lock-timeout、重复 archive/DB fault API 和冗余并发排列。