Document the trust model and the outer-clamp design - #728
Open
Gudge (MGudgin) wants to merge 3 commits into
Open
Document the trust model and the outer-clamp design#728Gudge (MGudgin) wants to merge 3 commits into
Gudge (MGudgin) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This was referenced Aug 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Documents MXC’s trust assumptions and proposed platform-specific outer-clamp model.
Changes:
- Defines the single-principal authorization model.
- Documents secure defaults, relaxation logging, and release-build safeguards.
- Compares potential clamp mechanisms across platforms.
Show a summary per file
| File | Description |
|---|---|
docs/versioning.md |
Adds the trust model and outer-clamp design. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (4)
docs/versioning.md:504
- This describes the pre-#726 logging path. The current #726 head routes relaxation records through
Logger::warning_line, which always writes to stderr, including successful default non-debug CLI runs. Leaving this limitation here would make the document stale as soon as the load-bearing stack is merged; the remaining gap is structured propagation through the in-process SDK.
configuration and infrastructure error paths. A successful default non-debug
run does not currently print the buffer, so this audit is not yet loud or
externally auditable in that mode. Filesystem path grants and the Seatbelt
docs/versioning.md:531
- This table now correctly says the Windows broker is not an independent clamp and that DACL mutation is enabled by default unless explicitly disabled (
FallbackPolicy::default()sets it totrue). The PR description still calls the broker row unbypassable and says DACL requires explicit opt-in, so the review/merge record contradicts both the document and implementation. Update the PR description to match these corrected rows.
| Windows — BaseContainer tier | None separate: the OS broker enforces the caller-authored sandbox spec | No independent ceiling | Current behavior; a fully relaxed config is passed to `Experimental_CreateProcessInSandbox` |
| Windows — AppContainer fallback tiers (BFS / DACL) | None separate: AppContainer, BFS, and DACL ACEs enforce the caller-authored paths | No independent ceiling | Current behavior; BFS is absent from a stock build and requires the non-default `tier2_bfs` feature plus `bfscfg.exe`, while DACL is the default stock fallback unless `fallback.allowDaclMutation=false` |
docs/versioning.md:535
- The “Unbypassable?” column applies inconsistent adversaries: the cross-platform file is marked bypassable because a local admin can edit it, while the LXC row is marked unbypassable even though host root can normally reconfigure its LSM/root-owned policy; SIP can likewise be disabled by a device owner with recovery access. Define whether host administrators are in scope, then classify every row consistently (for example, kernel-enforced against the sandboxed workload versus resistant to host-administrator changes).
| Linux (LXC) | Host LSM (AppArmor / SELinux) profile + root-owned policy file | Yes, with a host LSM; otherwise advisory | OS-infra / host config |
| Linux (Bubblewrap) | LSM, or compile-time capability removal | Partial — bwrap is unprivileged by design | OS-infra / build |
| macOS (Seatbelt) | Root-owned policy file + codesign / SIP | Yes, with SIP + signed binary; otherwise advisory | OS-infra (outside this repo) |
| All platforms | Root/admin-owned **clamp-policy file** read at the trust boundary, capping the boundary relaxations the parser accepts | No — a trust-boundary gate, bypassable by a local admin, not a kernel guarantee | Deferred (in-repo candidate) |
docs/versioning.md:510
- The current #726 head explicitly detects and warns when
blockedHostswidens networking on Hyperlight, NanVix, or applicable Bubblewrap configurations. The backend bugs tracked by #786/#787 remain, but the claim that the generic audit omits them will be false after this stack is merged.
`network.blockedHosts`: Hyperlight and NanVix currently interpret a non-empty
blocklist as allow-all-except even when `network.defaultPolicy=block`, while
the generic audit assumes blocklists only subtract connectivity and does not
log them. The backend fixes are tracked in
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This PR records the sandbox trust model and the platform-asymmetry clamp design in docs/versioning.md. It is documentation only: the mechanisms that would make an outer clamp genuinely unbypassable are OS infrastructure outside this repository, so it states the design honestly and defers enforcement rather than implying a guarantee that does not exist. Details * New "Trust model and the outer clamp" section describes the one-principal authorization model -- there is no mandatory second channel, because the same principal authors both the config and the command line -- and the three mechanisms that back it: secure defaults, loud logging of any relaxation, and rejection of catastrophic capabilities in shipped builds. * States the precondition explicitly: the model holds only where the invoker and the config author are the same trust principal. MXC does not enforce that, so an embedder passing a config derived from lower-trust input has two principals and must constrain the config itself. * Platform-asymmetry table covers the Windows broker, Linux LSM and root policy, Bubblewrap, macOS SIP and root policy, and an optional cross-platform in-repo clamp-policy file, each with whether it is truly unbypassable and its status. The Windows fallback row is qualified as absent from a stock build: BFS needs the non-default tier2_bfs feature plus bfscfg.exe, and the DACL path needs explicit config opt-in. * States plainly that the unbypassable rows (broker, LSM, SIP) are OS infrastructure out of scope for this repository, and that the in-repo clamp-file candidate would be a trust-boundary gate, not a kernel guarantee. Clamp enforcement itself is deferred. Tests * Documentation only; no code paths or CI gates are affected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8 Copilot-Session: cd48fff2-bde9-487a-ab67-012e9bbc0796
This PR corrects the trust-model documentation and records the micro-VM network-policy gaps. Details * Clarify that outer clamps are platform-dependent external bounds. * Correct the stock AppContainer DACL fallback behavior. * Document the blockedHosts audit gap and link the Hyperlight and NanVix bugs. Tests * `git diff --check` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4013d4d-fa64-4592-b0f7-93a492838954
This PR distinguishes diagnostic visibility and config enforcement from an independent outer clamp. Details * Document when relaxation audit entries are externally visible. * State that defaults and diagnostics do not cap caller authority. * Reclassify Windows enforcement tiers as having no separate clamp. Tests * `git diff --check` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4013d4d-fa64-4592-b0f7-93a492838954
Gudge (MGudgin)
force-pushed
the
user/gudge/trust-model-docs
branch
from
August 8, 2026 21:11
cbddcc7 to
af91203
Compare
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (4)
docs/versioning.md:502
- This describes the pre-#726 buffering behavior, but the current implementation emits every relaxation through
Logger::warning_line, which unconditionally writes to stderr even inMode::Buffer(src/core/wxc_common/src/logger.rs:256-260). Successful default non-debug CLI runs therefore do expose these warnings; documenting them as silent contradicts both the implementation and this PR's dependency.
`SECURITY: boundary relaxed: …`. They are immediately visible with `--debug`
or a configured diagnostic log sink, and the default CLIs print the buffer on
configuration and infrastructure error paths. A successful default non-debug
run does not currently print the buffer, so this audit is not yet loud or
externally auditable in that mode. Filesystem path grants and the Seatbelt
docs/versioning.md:534
- This marks the macOS design unbypassable without naming an OS primitive that protects either the arbitrary policy file or the third-party executable. SIP only clamps protected system paths (as documented in
docs/macos-support/seatbelt-backend.md:438-440), while code signing alone does not force callers to use that binary or prevent an administrator from replacing the policy/binary. That is the same bypass used to classify the all-platform policy-file row as non-kernel-enforced, so this row currently overstates the guarantee.
| macOS (Seatbelt) | Root-owned policy file + codesign / SIP | Yes, with SIP + signed binary; otherwise advisory | OS-infra (outside this repo) |
docs/versioning.md:508
- The generic audit no longer has this gap.
log_boundary_relaxationsexplicitly detectsblockedHostswithdefaultPolicy=blockfor Hyperlight and MicroVM and emits a warning (src/core/wxc_common/src/config_parser.rs:844-879), with regression coverage at lines 2160-2175. The backend enforcement bugs remain, but the claim that they are not logged is now inaccurate.
primary purpose rather than a relaxation. One known gap is
`network.blockedHosts`: Hyperlight and NanVix currently interpret a non-empty
blocklist as allow-all-except even when `network.defaultPolicy=block`, while
the generic audit assumes blocklists only subtract connectivity and does not
log them. The backend fixes are tracked in
docs/versioning.md:473
- This introductory sentence is a fragment because the subject has no main verb.
MXC's authorization model and the platform-dependent external bounds that can
constrain what a config relaxes.
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.github/copilot-instructions.md.Summary
This PR documents MXC's authorization model and the platform-dependent external bounds that can constrain it. It is documentation only: genuinely unbypassable clamps require OS or host infrastructure outside this repository, so the document states the current guarantees and gaps without implying an in-repo enforcement boundary that does not exist.
Details
--debug, a configured sink, or relevant error paths, but not after a successful default non-debug CLI run.fallback.allowDaclMutation=false.blockedHostsaudit/enforcement gaps and the corresponding follow-up issues.Tests
git diff --checkRelated Issues
Stack
Merge bottom-up; this document depends on the mechanisms introduced by the earlier PRs.
seatbelt.profileOverridein shipped builds