Conversation
📝 WalkthroughWalkthroughAdds sandboxing support using Landlock and a SandboxedCommand API in soar-core, bumps multiple crate versions across the workspace, adds changelog entries for v0.9.2/v0.?.x releases, and updates workspace dependencies (including adding the landlock crate). Changes
Sequence Diagram(s)sequenceDiagram
actor Caller
participant SandboxedCommand
participant LandlockAPI
participant ChildProcess
Caller ->> SandboxedCommand: build config (command, dirs, env, sandbox config)
Caller ->> SandboxedCommand: invoke run()
SandboxedCommand ->> LandlockAPI: query support / ABI
alt Landlock supported and sandbox enabled
SandboxedCommand ->> LandlockAPI: assemble rulesets (paths, network)
LandlockAPI -->> SandboxedCommand: ruleset ready
SandboxedCommand ->> ChildProcess: spawn with pre-exec hook (apply Landlock)
ChildProcess ->> LandlockAPI: apply restrictions then exec target command
ChildProcess -->> SandboxedCommand: exit status
else sandbox disabled or unsupported
SandboxedCommand ->> ChildProcess: spawn direct (no restrictions)
ChildProcess -->> SandboxedCommand: exit status
end
SandboxedCommand -->> Caller: return ExitStatus / Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/soar-registry/CHANGELOG.md (1)
12-12: Placeholder commit hash in existing entry.The 0.1.1 changelog entry references commit
0000000, which appears to be a placeholder. While this is pre-existing and not introduced by this PR, consider updating it to the actual commit hash for historical accuracy.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
CHANGELOG.mdCargo.tomlcrates/soar-cli/Cargo.tomlcrates/soar-core/CHANGELOG.mdcrates/soar-core/Cargo.tomlcrates/soar-db/CHANGELOG.mdcrates/soar-db/Cargo.tomlcrates/soar-dl/CHANGELOG.mdcrates/soar-dl/Cargo.tomlcrates/soar-package/CHANGELOG.mdcrates/soar-package/Cargo.tomlcrates/soar-registry/CHANGELOG.mdcrates/soar-registry/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (13)
crates/soar-cli/Cargo.toml (1)
3-3: LGTM!Version bump to 0.9.2 is consistent with the release PR objectives and the API-compatible patch increment for the error message refactor.
crates/soar-db/Cargo.toml (1)
3-3: LGTM!Version bump to 0.2.1 aligns with the workspace-wide release for the error message refactor.
crates/soar-db/CHANGELOG.md (1)
2-6: LGTM!Changelog entry correctly documents the 0.2.1 release with the refactor commit reference and proper comparison link.
crates/soar-core/CHANGELOG.md (1)
2-6: LGTM!Changelog entry for 0.10.1 is well-formatted and consistent with the workspace-wide release pattern.
crates/soar-dl/Cargo.toml (1)
3-3: LGTM!Version bump to 0.7.2 is consistent with the coordinated workspace release.
crates/soar-registry/CHANGELOG.md (1)
2-6: LGTM!Changelog entry for 0.1.2 correctly documents the refactor with the appropriate commit reference.
crates/soar-package/Cargo.toml (1)
3-3: LGTM!Version bump to 0.1.2 aligns with the coordinated workspace release.
crates/soar-core/Cargo.toml (1)
3-3: LGTM!Version bump to 0.10.1 is consistent with the workspace-wide release. Internal workspace dependencies will correctly resolve to the updated versions.
CHANGELOG.md (1)
2-7: LGTM! Changelog entry is well-formatted.The new version 0.9.2 changelog entry follows the conventional format with proper date, category, commit reference, and version comparison link. The refactor note aligns with the referenced commit.
crates/soar-dl/CHANGELOG.md (1)
2-7: LGTM! Changelog entry is consistent and properly formatted.The version 0.7.2 entry uses the correct crate-specific tag format and matches the refactor documented across all workspace crates in this release.
crates/soar-package/CHANGELOG.md (1)
2-7: LGTM! Changelog entry follows the correct format.The version 0.1.2 entry is properly formatted with the crate-specific tag prefix and documents the same refactor as other workspace crates.
crates/soar-registry/Cargo.toml (1)
3-3: LGTM! Version bump is correct.The version bump to 0.1.2 aligns with the PR objectives and represents an API-compatible patch release.
Cargo.toml (1)
51-55: LGTM! Workspace dependency versions are correctly synchronized.All workspace crate versions have been properly updated to match their individual package version bumps:
- soar-core: 0.10.0 → 0.10.1
- soar-db: 0.2.0 → 0.2.1
- soar-dl: 0.7.1 → 0.7.2
- soar-package: 0.1.1 → 0.1.2
- soar-registry: 0.1.1 → 0.1.2
These changes align with the PR objectives and maintain API compatibility across the workspace.
ca39b1c to
66609f3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @crates/soar-db/CHANGELOG.md:
- Around line 2-7: The changelog is missing a Breaking Changes section: add a
new "### ⚠️ Breaking Changes" entry for v0.3.0 that explicitly states the public
API structs NewPackage, PackageListing, and Package no longer include the
version_upstream field; explain the change, list the affected symbols
(NewPackage, PackageListing, Package, version_upstream), provide a brief
migration note instructing consumers to remove uses of version_upstream or map
it from their own metadata before constructing these structs, and include a
short example description of how to adapt callers (e.g., stop setting
version_upstream or populate an alternative field) so users upgrading from 0.2.0
to 0.3.0 have clear guidance.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
CHANGELOG.mdCargo.tomlcrates/soar-cli/Cargo.tomlcrates/soar-core/CHANGELOG.mdcrates/soar-core/Cargo.tomlcrates/soar-db/CHANGELOG.mdcrates/soar-db/Cargo.tomlcrates/soar-dl/CHANGELOG.mdcrates/soar-dl/Cargo.tomlcrates/soar-package/CHANGELOG.mdcrates/soar-package/Cargo.tomlcrates/soar-registry/CHANGELOG.mdcrates/soar-registry/Cargo.toml
✅ Files skipped from review due to trivial changes (2)
- crates/soar-core/Cargo.toml
- crates/soar-core/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (8)
- Cargo.toml
- crates/soar-dl/Cargo.toml
- crates/soar-package/CHANGELOG.md
- crates/soar-registry/CHANGELOG.md
- crates/soar-registry/Cargo.toml
- crates/soar-package/Cargo.toml
- CHANGELOG.md
- crates/soar-cli/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverage
- GitHub Check: test
🔇 Additional comments (3)
crates/soar-db/Cargo.toml (2)
3-3: LGTM! Version bump correctly reflects breaking changes.The minor version bump from 0.2.0 to 0.3.0 appropriately follows semantic versioning for pre-1.0 releases, where breaking changes increment the minor version. This aligns with the documented breaking changes in the PR objectives (field and struct removals).
16-26: Dependency management approach is appropriate.Using workspace-managed dependencies (
workspace = true) is the correct approach for coordinating version updates across the monorepo, especially when soar-registry (line 24) is also receiving breaking changes in this release.crates/soar-dl/CHANGELOG.md (1)
2-10: ✅ Changelog entry for soar-dl v0.7.2 is well-formatted.The changelog correctly documents an API-compatible release (0.7.1 → 0.7.2) with feature and refactor entries. The structure follows the established format with proper section headers, emojis, scopes, and commit references.
crates/soar-db/CHANGELOG.md
Outdated
| ## [0.3.0](https://github.com/pkgforge/soar/compare/soar-db-v0.2.0...soar-db-v0.3.0) - 2026-01-07 | ||
|
|
||
| ### 🚜 Refactor | ||
|
|
||
| - *(error)* Don't override error messages - ([e44342f](https://github.com/pkgforge/soar/commit/e44342f3c23b9cdbe23df2739bcf04bde4138025)) | ||
| - *(query)* Update query field icons - ([695a427](https://github.com/pkgforge/soar/commit/695a427ef6a4874cb212cdceed192f94150c5548)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the CHANGELOG.md file
cat -n crates/soar-db/CHANGELOG.md | head -20Repository: pkgforge/soar
Length of output: 1081
🏁 Script executed:
# Check the Cargo.toml for version information
cat crates/soar-db/Cargo.toml | grep -A 2 "^\[package\]"Repository: pkgforge/soar
Length of output: 101
🏁 Script executed:
# Search for the struct definitions mentioned in the review
rg "struct NewPackage|struct PackageListing|struct Package" crates/soar-db/src --type rustRepository: pkgforge/soar
Length of output: 635
🏁 Script executed:
# Check the NewPackage struct definition
sed -n '/pub struct NewPackage/,/^}/p' crates/soar-db/src/models/metadata.rsRepository: pkgforge/soar
Length of output: 1348
🏁 Script executed:
# Check the PackageListing struct definition
sed -n '/pub struct PackageListing/,/^}/p' crates/soar-db/src/models/metadata.rsRepository: pkgforge/soar
Length of output: 195
🏁 Script executed:
# Check the Package struct definition
sed -n '/pub struct Package/,/^}/p' crates/soar-db/src/models/metadata.rsRepository: pkgforge/soar
Length of output: 1620
🏁 Script executed:
# Search for any mention of version_upstream in the codebase to verify if it was removed
rg "version_upstream" crates/soar-db/Repository: pkgforge/soar
Length of output: 162
The version_upstream field has been removed from the public API structs (NewPackage, PackageListing, Package), but the changelog entry only lists refactors without documenting any breaking changes. Consumers upgrading from 0.2.0 to 0.3.0 need explicit visibility into this breaking API change before upgrading.
Add a ### ⚠️ Breaking Changes section documenting the removed field:
Suggested structure
## [0.3.0](https://github.com/pkgforge/soar/compare/soar-db-v0.2.0...soar-db-v0.3.0) - 2026-01-07
+### ⚠️ Breaking Changes
+
+- Removed `version_upstream` field from `NewPackage`, `PackageListing`, and `Package` structs
+
### 🚜 Refactor🤖 Prompt for AI Agents
In @crates/soar-db/CHANGELOG.md around lines 2 - 7, The changelog is missing a
Breaking Changes section: add a new "### ⚠️ Breaking Changes" entry for v0.3.0
that explicitly states the public API structs NewPackage, PackageListing, and
Package no longer include the version_upstream field; explain the change, list
the affected symbols (NewPackage, PackageListing, Package, version_upstream),
provide a brief migration note instructing consumers to remove uses of
version_upstream or map it from their own metadata before constructing these
structs, and include a short example description of how to adapt callers (e.g.,
stop setting version_upstream or populate an alternative field) so users
upgrading from 0.2.0 to 0.3.0 have clear guidance.
d078eee to
222278a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @crates/soar-registry/CHANGELOG.md:
- Around line 2-12: Add a "Breaking Changes" subsection to the v0.2.0 changelog
entry documenting that struct RemotePackage had three public fields removed;
explicitly list the removed fields `rank`, `pkg`, and `version_upstream` and
state that this is a breaking API change so downstream consumers are warned.
Ensure the new subsection appears under the v0.2.0 header and uses a clear label
such as "⚠️ Breaking Changes" with a short bullet describing the removed fields
from RemotePackage.
In @crates/soar-utils/CHANGELOG.md:
- Line 6: Replace the placeholder commit hash "0000000" used in the CHANGELOG.md
entry (e.g., the line "Update Cargo.toml dependencies - ([0000000](...))") with
the real 7-character commit hash used in this PR (verify and use the correct
hash such as af13bb6 or 222278a), and do the same replacement for the other
affected crate changelogs (soar-utils, soar-registry, soar-config) so the GitHub
links are valid; ensure each changelog link uses the exact 7-character commit id
in the markdown link.
🧹 Nitpick comments (1)
crates/soar-utils/CHANGELOG.md (1)
4-6: Consider adding more context to the changelog entry.The "Update Cargo.toml dependencies" entry is generic and doesn't explain what specifically changed in soar-utils or why the version was bumped. For a patch release (0.1.1 → 0.1.2), the changelog should clearly describe the functional or dependency changes involved.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
CHANGELOG.mdCargo.tomlcrates/soar-cli/Cargo.tomlcrates/soar-config/CHANGELOG.mdcrates/soar-config/Cargo.tomlcrates/soar-core/CHANGELOG.mdcrates/soar-core/Cargo.tomlcrates/soar-db/CHANGELOG.mdcrates/soar-db/Cargo.tomlcrates/soar-dl/CHANGELOG.mdcrates/soar-dl/Cargo.tomlcrates/soar-package/CHANGELOG.mdcrates/soar-package/Cargo.tomlcrates/soar-registry/CHANGELOG.mdcrates/soar-registry/Cargo.tomlcrates/soar-utils/CHANGELOG.mdcrates/soar-utils/Cargo.toml
✅ Files skipped from review due to trivial changes (1)
- crates/soar-core/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (13)
- crates/soar-cli/Cargo.toml
- crates/soar-package/CHANGELOG.md
- crates/soar-registry/Cargo.toml
- crates/soar-utils/Cargo.toml
- crates/soar-db/CHANGELOG.md
- crates/soar-config/Cargo.toml
- crates/soar-package/Cargo.toml
- crates/soar-dl/CHANGELOG.md
- CHANGELOG.md
- Cargo.toml
- crates/soar-dl/Cargo.toml
- crates/soar-db/Cargo.toml
- crates/soar-config/CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (1)
crates/soar-core/CHANGELOG.md (1)
2-13: Changelog entry is correct and properly formatted.All commit hashes exist in the repository, version numbering is appropriate (0.11.0 follows 0.10.0), the date is accurate, and the format is consistent with existing changelog entries. The features and refactors documented align with the PR scope.
crates/soar-registry/CHANGELOG.md
Outdated
| ## [0.2.0](https://github.com/pkgforge/soar/compare/soar-registry-v0.1.1...soar-registry-v0.2.0) - 2026-01-09 | ||
|
|
||
| ### ⛰️ Features | ||
|
|
||
| - *(apply)* Allow applying ghcr packages - ([06e2b73](https://github.com/pkgforge/soar/commit/06e2b73fce7f4189527b8868bb9adfe14d0600cc)) | ||
|
|
||
| ### 🚜 Refactor | ||
|
|
||
| - *(error)* Don't override error messages - ([e44342f](https://github.com/pkgforge/soar/commit/e44342f3c23b9cdbe23df2739bcf04bde4138025)) | ||
| - *(query)* Update query field icons - ([695a427](https://github.com/pkgforge/soar/commit/695a427ef6a4874cb212cdceed192f94150c5548)) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the struct RemotePackage to verify the breaking changes
find . -name "*.rs" -type f | xargs grep -l "struct RemotePackage" | head -5Repository: pkgforge/soar
Length of output: 94
🏁 Script executed:
# Check the CHANGELOG.md file in full to understand context
cat -n crates/soar-registry/CHANGELOG.md | head -50Repository: pkgforge/soar
Length of output: 1238
🏁 Script executed:
# Look for changelog patterns in other soar crates
fd "CHANGELOG.md" --type f | head -10Repository: pkgforge/soar
Length of output: 285
🏁 Script executed:
# Search for any documentation of breaking changes in the commits or recent changes
git log --oneline --all -- crates/soar-registry/CHANGELOG.md | head -10Repository: pkgforge/soar
Length of output: 139
🏁 Script executed:
# Examine the struct RemotePackage definition
cat -n crates/soar-registry/src/package.rs | head -100Repository: pkgforge/soar
Length of output: 3542
🏁 Script executed:
# Check other crate changelogs for breaking changes patterns
cat crates/soar-core/CHANGELOG.md | grep -A 10 "Breaking\|⚠️" | head -30Repository: pkgforge/soar
Length of output: 39
🏁 Script executed:
# Check soar-package changelog
cat crates/soar-package/CHANGELOG.md | grep -A 10 "Breaking\|⚠️" | head -30Repository: pkgforge/soar
Length of output: 39
🏁 Script executed:
# Search for any mentions of RemotePackage changes in recent commits
git log --all --oneline --grep="RemotePackage\|breaking\|Breaking" | head -20Repository: pkgforge/soar
Length of output: 108
🏁 Script executed:
# Get more of the package.rs file to find RemotePackage struct definition
wc -l crates/soar-registry/src/package.rsRepository: pkgforge/soar
Length of output: 96
🏁 Script executed:
# Read the RemotePackage struct definition
sed -n '100,250p' crates/soar-registry/src/package.rsRepository: pkgforge/soar
Length of output: 4497
🏁 Script executed:
# Search for mentions of rank, pkg, version_upstream fields
rg "rank|pkg|version_upstream" crates/soar-registry/src/package.rs -C 3Repository: pkgforge/soar
Length of output: 1945
🏁 Script executed:
# Check git history for this file to see what changed between versions
git log --oneline --all -- crates/soar-registry/src/package.rs | head -20Repository: pkgforge/soar
Length of output: 255
🏁 Script executed:
# Check if there's a PR description or PR file
git log --all --oneline | grep -i "0.2.0\|release" | head -10Repository: pkgforge/soar
Length of output: 394
🏁 Script executed:
# Check git diff between v0.1.1 and v0.2.0 tags or commits for RemotePackage
git tag | grep "soar-registry" | sort -VRepository: pkgforge/soar
Length of output: 39
🏁 Script executed:
# Look for any version tags
git log --all --oneline --decorate | grep -i "soar-registry\|0.2.0\|0.1.1" | head -20Repository: pkgforge/soar
Length of output: 148
🏁 Script executed:
# Check the actual current state of the code - are there ANY removed fields somewhere?
rg "pub.*rank|pub.*version_upstream" crates/soar-registry/ -t rustRepository: pkgforge/soar
Length of output: 39
🏁 Script executed:
# Check the release commit to see if it includes any documentation of changes
git show 5831e60 | head -100Repository: pkgforge/soar
Length of output: 2337
🏁 Script executed:
# Check if there's any documentation file mentioning breaking changes
find . -name "*.md" -o -name "*.txt" | xargs grep -l "breaking\|BREAKING" 2>/dev/null | head -10Repository: pkgforge/soar
Length of output: 71
🏁 Script executed:
# Search entire repo for any mention of fields: rank, pkg (as field name), version_upstream
rg "\.rank|\.pkg(?![_a-z])|\.version_upstream" crates/soar-registry/ -t rustRepository: pkgforge/soar
Length of output: 313
🏁 Script executed:
# Check the diff between init commit and current to see if RemotePackage changed
git show 21070db:crates/soar-registry/src/package.rs 2>/dev/null | grep -A 50 "struct RemotePackage"Repository: pkgforge/soar
Length of output: 1568
Document breaking API changes in the changelog.
The v0.2.0 release removes three public fields from struct RemotePackage: rank, pkg, and version_upstream. These are breaking changes that affect any downstream consumers using these fields. The current changelog entry omits this critical information.
Add a "Breaking Changes" section to the v0.2.0 changelog entry:
Suggested changelog addition
### ⚠️ Breaking Changes
- Struct `RemotePackage`: removed public fields (`rank`, `pkg`, `version_upstream`)🤖 Prompt for AI Agents
In @crates/soar-registry/CHANGELOG.md around lines 2 - 12, Add a "Breaking
Changes" subsection to the v0.2.0 changelog entry documenting that struct
RemotePackage had three public fields removed; explicitly list the removed
fields `rank`, `pkg`, and `version_upstream` and state that this is a breaking
API change so downstream consumers are warned. Ensure the new subsection appears
under the v0.2.0 header and uses a clear label such as "⚠️ Breaking Changes"
with a short bullet describing the removed fields from RemotePackage.
222278a to
46fabcb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @Cargo.toml:
- Line 37: The Cargo.toml dependency for the landlock crate is set to a
non-existent version ("0.4.4"); update the landlock entry in Cargo.toml (the
line containing landlock = "...") to use the published version "0.4.2" (or
confirm and pin the intended released version) so the project builds
successfully.
In @crates/soar-core/src/sandbox.rs:
- Around line 358-424: The doc claims apply_landlock_restrictions is
async-signal-safe but it calls the tracing macro debug! (at the end of
apply_landlock_restrictions), which is not guaranteed signal-safe; remove the
debug! call from apply_landlock_restrictions (or move logging to the caller
after exec returns) and update the function doc comment to either drop the
async-signal-safe claim or explicitly note that logging is omitted within the
pre_exec path to preserve signal-safety.
🧹 Nitpick comments (2)
crates/soar-core/src/sandbox.rs (2)
261-278: Consider adding/dev/shmto default write paths.Some programs require
/dev/shmfor POSIX shared memory operations. If sandboxed commands fail due to missing shared memory access, users would need to explicitly add this path.This is a minor observation—the current defaults are reasonable, and users can add extra paths via the builder API.
Optional addition
fn default_write_paths() -> Vec<PathBuf> { [ "/dev/null", "/dev/zero", "/dev/tty", "/dev/stdin", "/dev/stdout", "/dev/stderr", "/dev/fd", "/dev/pts", "/dev/ptmx", "/tmp", + "/dev/shm", ] .iter() .map(PathBuf::from) .collect() }
117-128: Silent fallback to/whencurrent_dir()fails.Line 122 silently falls back to
/as the working directory ifstd::env::current_dir()fails. This could lead to unexpected behavior. Consider logging a warning when this occurs.Optional: Add warning on fallback
pub fn new(command: &'a str) -> Self { + let working_dir = std::env::current_dir().unwrap_or_else(|e| { + tracing::warn!("Failed to get current directory, using '/': {}", e); + PathBuf::from("/") + }); Self { command, - working_dir: std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), + working_dir, env_vars: Vec::new(), config: SandboxConfig::new(), extra_read_paths: Vec::new(), extra_write_paths: Vec::new(), } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
CHANGELOG.mdCargo.tomlcrates/soar-cli/Cargo.tomlcrates/soar-config/CHANGELOG.mdcrates/soar-config/Cargo.tomlcrates/soar-core/CHANGELOG.mdcrates/soar-core/Cargo.tomlcrates/soar-core/src/lib.rscrates/soar-core/src/sandbox.rscrates/soar-db/CHANGELOG.mdcrates/soar-db/Cargo.tomlcrates/soar-dl/CHANGELOG.mdcrates/soar-dl/Cargo.tomlcrates/soar-package/CHANGELOG.mdcrates/soar-package/Cargo.tomlcrates/soar-registry/CHANGELOG.mdcrates/soar-registry/Cargo.tomlcrates/soar-utils/CHANGELOG.mdcrates/soar-utils/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (11)
- crates/soar-db/Cargo.toml
- crates/soar-config/CHANGELOG.md
- CHANGELOG.md
- crates/soar-db/CHANGELOG.md
- crates/soar-core/Cargo.toml
- crates/soar-dl/Cargo.toml
- crates/soar-config/Cargo.toml
- crates/soar-cli/Cargo.toml
- crates/soar-utils/CHANGELOG.md
- crates/soar-package/Cargo.toml
- crates/soar-registry/Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T16:14:13.649Z
Learnt from: QaidVoid
Repo: pkgforge/soar PR: 137
File: crates/soar-cli/src/apply.rs:149-153
Timestamp: 2026-01-09T16:14:13.649Z
Learning: In crates/soar-cli/src/apply.rs, for URL packages, passing non-installed records (is_installed=false) as existing_install is intentional. The query filters by repo_name, pkg_name, and pkg_id, ensuring the record is for the same package. This allows resuming partial installs and preserving settings (portable paths, profile) from previous installs.
Applied to files:
crates/soar-package/CHANGELOG.md
🔇 Additional comments (10)
crates/soar-dl/CHANGELOG.md (1)
2-11: LGTM! Changelog entry is well-formatted.The new v0.7.2 changelog entry follows the established format, correctly documents the features and refactors, and includes proper commit references.
crates/soar-registry/CHANGELOG.md (1)
2-12: LGTM! Changelog entry is well-formatted.The new v0.2.0 changelog entry correctly documents the features and refactors with proper commit references and follows the established format.
crates/soar-package/CHANGELOG.md (1)
2-12: LGTM! Changelog entry is well-formatted.The new v0.2.0 changelog entry properly documents the features (including the new entrypoint option and executable discovery fallbacks) and refactors with valid commit references.
Cargo.toml (1)
51-57: Version bumps are consistent with documented semver changes.The workspace dependency versions correctly reflect the breaking vs. compatible changes documented in the PR objectives:
- Breaking API changes (soar-config, soar-registry, soar-db, soar-package) → minor/major bumps
- Compatible changes (soar-utils, soar-dl, soar-core) → patch bumps
crates/soar-utils/Cargo.toml (1)
3-3: LGTM!Version bump from 0.1.1 to 0.1.2 is consistent with the API-compatible changes documented in the PR.
crates/soar-core/CHANGELOG.md (1)
2-15: LGTM!Changelog entry for v0.10.1 is well-structured with clear categorization of features and refactors. The entries align with the documented changes in this release.
crates/soar-core/src/lib.rs (1)
7-7: LGTM!The new
sandboxmodule is correctly exported as a public module, expanding the crate's API surface to include Landlock-based sandboxing capabilities.crates/soar-core/src/sandbox.rs (3)
489-494: LGTM on pre-allocation strategy.The paths and network config are correctly pre-allocated before entering the
pre_execcontext (lines 465-480), and the closure captures them by move. This avoids heap allocations in the forked child process.
17-44: LGTM!The
NetworkConfigAPI is well-designed with clear semantics:
allow_all()bypasses network restrictions entirelyallow_https()provides a sensible default for HTTPS-only accessThe implementation correctly skips network rule setup when
allow_allis true (lines 391, 411).
436-461: Graceful degradation when Landlock is unavailable.The fallback behavior (running without sandbox when Landlock isn't supported) is appropriate for maintaining functionality on older kernels. The warning log ensures users are aware of the reduced security posture.
| documented = "0.9.2" | ||
| fast-glob = "1.0.0" | ||
| image = { version = "0.25.9", default-features = false, features = ["png"] } | ||
| landlock = "0.4.4" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
landlock rust crate latest version crates.io
💡 Result:
The latest released version is 0.4.2 — published April 29, 2025. [1][2]
Sources:
[1] docs.rs crate page for landlock.
[2] lib.rs / changelog listing (landlock 0.4.2, Apr 29, 2025).
Update landlock dependency version to 0.4.2.
The specified version 0.4.4 does not match the latest available release. The latest version of the landlock crate is 0.4.2 (released April 29, 2025). Using a non-existent version will cause build failures. Update to landlock = "0.4.2" or verify that 0.4.4 has been released if this is intentional.
🤖 Prompt for AI Agents
In @Cargo.toml at line 37, The Cargo.toml dependency for the landlock crate is
set to a non-existent version ("0.4.4"); update the landlock entry in Cargo.toml
(the line containing landlock = "...") to use the published version "0.4.2" (or
confirm and pin the intended released version) so the project builds
successfully.
| /// Apply Landlock restrictions to the current process. | ||
| /// | ||
| /// This function is designed to be called from a `pre_exec` hook, which runs | ||
| /// in the forked child process before exec(). It sets up filesystem and | ||
| /// optionally network restrictions using the Landlock LSM. | ||
| /// | ||
| /// # Signal Safety | ||
| /// | ||
| /// This function is async-signal-safe because: | ||
| /// - Landlock operations are direct syscalls (landlock_create_ruleset, | ||
| /// landlock_add_rule, landlock_restrict_self) | ||
| /// - No heap allocations occur after the function starts (all data is pre-allocated) | ||
| /// - No locks are acquired | ||
| /// - No async-signal-unsafe functions are called | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `read_paths` - Paths to allow read access | ||
| /// * `write_paths` - Paths to allow full (read/write) access | ||
| /// * `network_config` - Network restriction configuration | ||
| fn apply_landlock_restrictions( | ||
| read_paths: &[PathBuf], | ||
| write_paths: &[PathBuf], | ||
| network_config: &NetworkConfig, | ||
| ) -> std::io::Result<()> { | ||
| let abi = get_best_abi(); | ||
| let read_access = AccessFs::from_read(abi); | ||
| let write_access = AccessFs::from_all(abi); | ||
|
|
||
| let ruleset_builder = Ruleset::default() | ||
| .handle_access(AccessFs::from_all(abi)) | ||
| .map_err(|e| std::io::Error::other(format!("Landlock FS access setup failed: {}", e)))?; | ||
|
|
||
| let ruleset_builder = if is_network_supported(abi) && !network_config.allow_all { | ||
| ruleset_builder | ||
| .handle_access(AccessNet::from_all(abi)) | ||
| .map_err(|e| { | ||
| std::io::Error::other(format!("Landlock network access setup failed: {}", e)) | ||
| })? | ||
| } else { | ||
| ruleset_builder | ||
| }; | ||
|
|
||
| let ruleset = ruleset_builder | ||
| .create() | ||
| .map_err(|e| std::io::Error::other(format!("Landlock ruleset creation failed: {}", e)))?; | ||
|
|
||
| let ruleset = | ||
| add_path_rules(ruleset, read_paths, read_access).map_err(std::io::Error::other)?; | ||
|
|
||
| let ruleset = | ||
| add_path_rules(ruleset, write_paths, write_access).map_err(std::io::Error::other)?; | ||
|
|
||
| let ruleset = if is_network_supported(abi) && !network_config.allow_all { | ||
| add_network_rules(ruleset, network_config).map_err(std::io::Error::other)? | ||
| } else { | ||
| ruleset | ||
| }; | ||
|
|
||
| // Enforce the sandbox | ||
| ruleset | ||
| .restrict_self() | ||
| .map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?; | ||
|
|
||
| debug!("Landlock sandbox activated"); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Signal safety concern: debug! macro may not be async-signal-safe.
The doc comment claims async-signal-safety, but apply_landlock_restrictions calls debug!() at line 422. The tracing macros typically involve locks and allocations internally, which violates signal safety guarantees in the pre_exec context.
Consider removing the debug! call from within this function, or updating the documentation to reflect that full signal safety is not guaranteed (though in practice, the risk is low since Landlock syscalls complete before the log).
Suggested documentation update
/// # Signal Safety
///
-/// This function is async-signal-safe because:
-/// - Landlock operations are direct syscalls (landlock_create_ruleset,
-/// landlock_add_rule, landlock_restrict_self)
-/// - No heap allocations occur after the function starts (all data is pre-allocated)
-/// - No locks are acquired
-/// - No async-signal-unsafe functions are called
+/// This function performs Landlock syscalls which are signal-safe. However,
+/// the debug logging at the end is not strictly async-signal-safe. In practice,
+/// this is unlikely to cause issues as it occurs after sandbox enforcement.Or remove the debug call:
// Enforce the sandbox
ruleset
.restrict_self()
.map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?;
- debug!("Landlock sandbox activated");
Ok(())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Apply Landlock restrictions to the current process. | |
| /// | |
| /// This function is designed to be called from a `pre_exec` hook, which runs | |
| /// in the forked child process before exec(). It sets up filesystem and | |
| /// optionally network restrictions using the Landlock LSM. | |
| /// | |
| /// # Signal Safety | |
| /// | |
| /// This function is async-signal-safe because: | |
| /// - Landlock operations are direct syscalls (landlock_create_ruleset, | |
| /// landlock_add_rule, landlock_restrict_self) | |
| /// - No heap allocations occur after the function starts (all data is pre-allocated) | |
| /// - No locks are acquired | |
| /// - No async-signal-unsafe functions are called | |
| /// | |
| /// # Arguments | |
| /// | |
| /// * `read_paths` - Paths to allow read access | |
| /// * `write_paths` - Paths to allow full (read/write) access | |
| /// * `network_config` - Network restriction configuration | |
| fn apply_landlock_restrictions( | |
| read_paths: &[PathBuf], | |
| write_paths: &[PathBuf], | |
| network_config: &NetworkConfig, | |
| ) -> std::io::Result<()> { | |
| let abi = get_best_abi(); | |
| let read_access = AccessFs::from_read(abi); | |
| let write_access = AccessFs::from_all(abi); | |
| let ruleset_builder = Ruleset::default() | |
| .handle_access(AccessFs::from_all(abi)) | |
| .map_err(|e| std::io::Error::other(format!("Landlock FS access setup failed: {}", e)))?; | |
| let ruleset_builder = if is_network_supported(abi) && !network_config.allow_all { | |
| ruleset_builder | |
| .handle_access(AccessNet::from_all(abi)) | |
| .map_err(|e| { | |
| std::io::Error::other(format!("Landlock network access setup failed: {}", e)) | |
| })? | |
| } else { | |
| ruleset_builder | |
| }; | |
| let ruleset = ruleset_builder | |
| .create() | |
| .map_err(|e| std::io::Error::other(format!("Landlock ruleset creation failed: {}", e)))?; | |
| let ruleset = | |
| add_path_rules(ruleset, read_paths, read_access).map_err(std::io::Error::other)?; | |
| let ruleset = | |
| add_path_rules(ruleset, write_paths, write_access).map_err(std::io::Error::other)?; | |
| let ruleset = if is_network_supported(abi) && !network_config.allow_all { | |
| add_network_rules(ruleset, network_config).map_err(std::io::Error::other)? | |
| } else { | |
| ruleset | |
| }; | |
| // Enforce the sandbox | |
| ruleset | |
| .restrict_self() | |
| .map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?; | |
| debug!("Landlock sandbox activated"); | |
| Ok(()) | |
| } | |
| /// Apply Landlock restrictions to the current process. | |
| /// | |
| /// This function is designed to be called from a `pre_exec` hook, which runs | |
| /// in the forked child process before exec(). It sets up filesystem and | |
| /// optionally network restrictions using the Landlock LSM. | |
| /// | |
| /// # Signal Safety | |
| /// | |
| /// This function performs Landlock syscalls which are signal-safe. However, | |
| /// the debug logging at the end is not strictly async-signal-safe. In practice, | |
| /// this is unlikely to cause issues as it occurs after sandbox enforcement. | |
| /// | |
| /// # Arguments | |
| /// | |
| /// * `read_paths` - Paths to allow read access | |
| /// * `write_paths` - Paths to allow full (read/write) access | |
| /// * `network_config` - Network restriction configuration | |
| fn apply_landlock_restrictions( | |
| read_paths: &[PathBuf], | |
| write_paths: &[PathBuf], | |
| network_config: &NetworkConfig, | |
| ) -> std::io::Result<()> { | |
| let abi = get_best_abi(); | |
| let read_access = AccessFs::from_read(abi); | |
| let write_access = AccessFs::from_all(abi); | |
| let ruleset_builder = Ruleset::default() | |
| .handle_access(AccessFs::from_all(abi)) | |
| .map_err(|e| std::io::Error::other(format!("Landlock FS access setup failed: {}", e)))?; | |
| let ruleset_builder = if is_network_supported(abi) && !network_config.allow_all { | |
| ruleset_builder | |
| .handle_access(AccessNet::from_all(abi)) | |
| .map_err(|e| { | |
| std::io::Error::other(format!("Landlock network access setup failed: {}", e)) | |
| })? | |
| } else { | |
| ruleset_builder | |
| }; | |
| let ruleset = ruleset_builder | |
| .create() | |
| .map_err(|e| std::io::Error::other(format!("Landlock ruleset creation failed: {}", e)))?; | |
| let ruleset = | |
| add_path_rules(ruleset, read_paths, read_access).map_err(std::io::Error::other)?; | |
| let ruleset = | |
| add_path_rules(ruleset, write_paths, write_access).map_err(std::io::Error::other)?; | |
| let ruleset = if is_network_supported(abi) && !network_config.allow_all { | |
| add_network_rules(ruleset, network_config).map_err(std::io::Error::other)? | |
| } else { | |
| ruleset | |
| }; | |
| // Enforce the sandbox | |
| ruleset | |
| .restrict_self() | |
| .map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?; | |
| debug!("Landlock sandbox activated"); | |
| Ok(()) | |
| } |
| /// Apply Landlock restrictions to the current process. | |
| /// | |
| /// This function is designed to be called from a `pre_exec` hook, which runs | |
| /// in the forked child process before exec(). It sets up filesystem and | |
| /// optionally network restrictions using the Landlock LSM. | |
| /// | |
| /// # Signal Safety | |
| /// | |
| /// This function is async-signal-safe because: | |
| /// - Landlock operations are direct syscalls (landlock_create_ruleset, | |
| /// landlock_add_rule, landlock_restrict_self) | |
| /// - No heap allocations occur after the function starts (all data is pre-allocated) | |
| /// - No locks are acquired | |
| /// - No async-signal-unsafe functions are called | |
| /// | |
| /// # Arguments | |
| /// | |
| /// * `read_paths` - Paths to allow read access | |
| /// * `write_paths` - Paths to allow full (read/write) access | |
| /// * `network_config` - Network restriction configuration | |
| fn apply_landlock_restrictions( | |
| read_paths: &[PathBuf], | |
| write_paths: &[PathBuf], | |
| network_config: &NetworkConfig, | |
| ) -> std::io::Result<()> { | |
| let abi = get_best_abi(); | |
| let read_access = AccessFs::from_read(abi); | |
| let write_access = AccessFs::from_all(abi); | |
| let ruleset_builder = Ruleset::default() | |
| .handle_access(AccessFs::from_all(abi)) | |
| .map_err(|e| std::io::Error::other(format!("Landlock FS access setup failed: {}", e)))?; | |
| let ruleset_builder = if is_network_supported(abi) && !network_config.allow_all { | |
| ruleset_builder | |
| .handle_access(AccessNet::from_all(abi)) | |
| .map_err(|e| { | |
| std::io::Error::other(format!("Landlock network access setup failed: {}", e)) | |
| })? | |
| } else { | |
| ruleset_builder | |
| }; | |
| let ruleset = ruleset_builder | |
| .create() | |
| .map_err(|e| std::io::Error::other(format!("Landlock ruleset creation failed: {}", e)))?; | |
| let ruleset = | |
| add_path_rules(ruleset, read_paths, read_access).map_err(std::io::Error::other)?; | |
| let ruleset = | |
| add_path_rules(ruleset, write_paths, write_access).map_err(std::io::Error::other)?; | |
| let ruleset = if is_network_supported(abi) && !network_config.allow_all { | |
| add_network_rules(ruleset, network_config).map_err(std::io::Error::other)? | |
| } else { | |
| ruleset | |
| }; | |
| // Enforce the sandbox | |
| ruleset | |
| .restrict_self() | |
| .map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?; | |
| debug!("Landlock sandbox activated"); | |
| Ok(()) | |
| } | |
| /// Apply Landlock restrictions to the current process. | |
| /// | |
| /// This function is designed to be called from a `pre_exec` hook, which runs | |
| /// in the forked child process before exec(). It sets up filesystem and | |
| /// optionally network restrictions using the Landlock LSM. | |
| /// | |
| /// # Signal Safety | |
| /// | |
| /// This function is async-signal-safe because: | |
| /// - Landlock operations are direct syscalls (landlock_create_ruleset, | |
| /// landlock_add_rule, landlock_restrict_self) | |
| /// - No heap allocations occur after the function starts (all data is pre-allocated) | |
| /// - No locks are acquired | |
| /// - No async-signal-unsafe functions are called | |
| /// | |
| /// # Arguments | |
| /// | |
| /// * `read_paths` - Paths to allow read access | |
| /// * `write_paths` - Paths to allow full (read/write) access | |
| /// * `network_config` - Network restriction configuration | |
| fn apply_landlock_restrictions( | |
| read_paths: &[PathBuf], | |
| write_paths: &[PathBuf], | |
| network_config: &NetworkConfig, | |
| ) -> std::io::Result<()> { | |
| let abi = get_best_abi(); | |
| let read_access = AccessFs::from_read(abi); | |
| let write_access = AccessFs::from_all(abi); | |
| let ruleset_builder = Ruleset::default() | |
| .handle_access(AccessFs::from_all(abi)) | |
| .map_err(|e| std::io::Error::other(format!("Landlock FS access setup failed: {}", e)))?; | |
| let ruleset_builder = if is_network_supported(abi) && !network_config.allow_all { | |
| ruleset_builder | |
| .handle_access(AccessNet::from_all(abi)) | |
| .map_err(|e| { | |
| std::io::Error::other(format!("Landlock network access setup failed: {}", e)) | |
| })? | |
| } else { | |
| ruleset_builder | |
| }; | |
| let ruleset = ruleset_builder | |
| .create() | |
| .map_err(|e| std::io::Error::other(format!("Landlock ruleset creation failed: {}", e)))?; | |
| let ruleset = | |
| add_path_rules(ruleset, read_paths, read_access).map_err(std::io::Error::other)?; | |
| let ruleset = | |
| add_path_rules(ruleset, write_paths, write_access).map_err(std::io::Error::other)?; | |
| let ruleset = if is_network_supported(abi) && !network_config.allow_all { | |
| add_network_rules(ruleset, network_config).map_err(std::io::Error::other)? | |
| } else { | |
| ruleset | |
| }; | |
| // Enforce the sandbox | |
| ruleset | |
| .restrict_self() | |
| .map_err(|e| std::io::Error::other(format!("Failed to enforce Landlock: {}", e)))?; | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In @crates/soar-core/src/sandbox.rs around lines 358 - 424, The doc claims
apply_landlock_restrictions is async-signal-safe but it calls the tracing macro
debug! (at the end of apply_landlock_restrictions), which is not guaranteed
signal-safe; remove the debug! call from apply_landlock_restrictions (or move
logging to the caller after exec returns) and update the function doc comment to
either drop the async-signal-safe claim or explicitly note that logging is
omitted within the pre_exec path to preserve signal-safety.
🤖 New release
soar-utils: 0.1.1 -> 0.1.2 (✓ API compatible changes)soar-config: 0.1.1 -> 0.2.0 (⚠ API breaking changes)soar-dl: 0.7.1 -> 0.7.2 (✓ API compatible changes)soar-registry: 0.1.1 -> 0.2.0 (⚠ API breaking changes)soar-db: 0.2.0 -> 0.3.0 (⚠ API breaking changes)soar-package: 0.1.1 -> 0.2.0 (⚠ API breaking changes)soar-core: 0.10.0 -> 0.10.1 (✓ API compatible changes)soar-cli: 0.9.1 -> 0.9.2⚠
soar-configbreaking changes⚠
soar-registrybreaking changes⚠
soar-dbbreaking changes⚠
soar-packagebreaking changesChangelog
soar-utilssoar-configsoar-dlsoar-registrysoar-dbsoar-packagesoar-coresoar-cliThis PR was generated with release-plz.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.