Make seatbelt.profileOverride dev-only (rejected by shipped builds) - #727
Open
Gudge (MGudgin) wants to merge 2 commits into
Open
Conversation
This PR adds a deterministic, centralized diagnostic warning for every config setting that opens a security boundary beyond the secure default, so weakening the sandbox is loud and auditable in the log rather than silently honored. It is logging only: no behavior, defaults, or enforcement change. Details * log_boundary_relaxations() in config_parser.rs emits one `SECURITY: boundary relaxed: <field>` line per relaxation, called once after the domain policy is built so every backend is covered from a single place. * Covers network (defaultPolicy=allow, allowLocalNetwork, allowedHosts, proxy), ui (clipboard, and disable=false plus the settings it makes effective), processContainer (capabilities, ui isolation/desktopSystemControl/ systemSettings/ime), and seatbelt (launchMethod=open, profileOverride, guiAccess, keychainAccess, extraMachLookups). * ui.clipboard is reported regardless of ui.disable. The Seatbelt profile builder emits the pasteboard mach-lookup grant from ui.clipboard alone, outside its ui.disable branch, so a UI-disabled macOS sandbox with clipboard enabled still has a live pasteboard channel. The warning is redundant on Windows, where a disabled UI forces clipboard blocks, but it never misses a real relaxation. * seatbelt.launchMethod=open is reported: it sandboxes the inner command rather than the launched app, which moves the trust boundary. * Free-form values are escaped and length-capped before they reach the log, so a newline in a config value cannot forge additional `SECURITY: boundary relaxed:` lines and poison the audit stream. * blockedHosts is deliberately not reported: it only subtracts connectivity, so it never relaxes the boundary. Settings that are genuinely inert under a stricter parent are also suppressed -- injection and the BaseProcess desktop knobs only warn when ui.disable=false makes them effective. * Filesystem grants and seatbelt nestedPty are out of scope: they are the request's primary purpose, not a relaxation. Tests * Five unit tests: the base relaxed/secure-default pair; clipboard_warns_even_when_ui_is_disabled, which pins the Seatbelt behavior above; seatbelt_launch_method_open_warns; mixed_policy_suppresses_only_the_ inert_field, which proves suppression is field-specific rather than a blanket loss of logging; and free_form_log_values_cannot_forge_audit_lines. * cargo test -p wxc_common: 560 passed, 0 failed. * cargo fmt --all -- --check and cargo clippy --workspace --all-targets -D warnings both clean. 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 removes the catastrophic seatbelt.profileOverride escape hatch from release binaries. profileOverride replaces the entire generated deny-default Seatbelt profile with a caller-supplied string, so it must not be honorable in a shipped build. Release builds reject a config that sets it, with defense in depth at both the parse and build layers. Details * config_parser make_seatbelt_config returns a config error in release builds when profile_override is set, and logs a SECURITY line naming the field. Rejecting rather than dropping the field means a caller never runs under a policy they did not request: the generated profile can be materially more permissive than the custom one they supplied. This matches how the reserved learning-mode capabilities are handled. * profile_builder build_profile compiles the override branch out entirely under #[cfg(not(debug_assertions))], so a release binary builds the generated deny-default profile even if it were reached with an override set. The parser rejection alone would be a single point of failure. * Dev and debug builds honor the override for advanced testing. * Docs (seatbelt-backend.md, schema.md) and the wire-model doc comment describe the field as rejected by release builds. schema.md records that the released stable schemas describe it as generally usable, since those files are immutable and cannot be corrected in place. Tests * Release: seatbelt_profile_override_rejected_in_release asserts the parse fails, that the error names the field and states the dev-only rejection, and that a SECURITY line is logged. profile_override_branch_is_absent_in_release asserts the builder returns the generated deny-default profile rather than the caller's string, and that the request policy is reflected in it. * Debug: profile_override_passed_through_in_debug and profile_override_takes_precedence assert the dev behavior. * cargo test -p wxc_common -p seatbelt_common passes in both profiles: 555 + 36 in debug, 555 + 36 in release, 0 failed. * cargo fmt --all -- --check and cargo clippy --workspace --all-targets -D warnings both clean; codegen gates green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8 Copilot-Session: cd48fff2-bde9-487a-ab67-012e9bbc0796
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This was referenced Aug 1, 2026
| | Field | Type | Default | Description | | ||
| |---|---|---|---| | ||
| | `seatbelt.profileOverride` | string | unset | Optional override of the generated TinyScheme sandbox profile. When set, the SDK-generated profile is replaced with this raw TinyScheme string verbatim — all `filesystem`/`network`/`ui` policy fields are ignored for profile generation (they are still type-checked). Use this only when the auto-generated profile is insufficient. | | ||
| | `seatbelt.profileOverride` | string | unset | **Dev-only.** Optional override of the generated TinyScheme sandbox profile. When set, the generated profile is replaced with this raw TinyScheme string verbatim. Because it bypasses the deny-default profile entirely, it is a catastrophic escape hatch: **release/shipped builds strip it at parse time (logging a `SECURITY` line) and compile the override path out**, so it is honored only in dev/debug builds. | |
Collaborator
There was a problem hiding this comment.
thought: yea I guess the only usage for this would be to debug our profile generation and for testing. Then again, this is probably the only backend config with testing specific configuration. Wonder if it is worth it to remove it entirely.
Branden Bonaby (bbonaby)
previously approved these changes
Aug 3, 2026
Gudge (MGudgin)
dismissed
Branden Bonaby (bbonaby)’s stale review
August 4, 2026 00:55
The merge-base changed after approval.
Collaborator
|
Gudge (@MGudgin), just have a merge conflict with this one. |
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
2 participants
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.
This PR removes the catastrophic seatbelt.profileOverride escape hatch from
release binaries. profileOverride replaces the entire generated deny-default
Seatbelt profile with a caller-supplied string, so it must not be honorable in a
shipped build. Release builds reject a config that sets it, with defense in depth
at both the parse and build layers.
Details
when profile_override is set, and logs a SECURITY line naming the field.
Rejecting rather than dropping the field means a caller never runs under a
policy they did not request: the generated profile can be materially more
permissive than the custom one they supplied. This matches how the reserved
learning-mode capabilities are handled.
#[cfg(not(debug_assertions))], so a release binary builds the generated
deny-default profile even if it were reached with an override set. The parser
rejection alone would be a single point of failure.
the field as rejected by release builds. schema.md records that the released
stable schemas describe it as generally usable, since those files are immutable
and cannot be corrected in place.
Tests
that the error names the field and states the dev-only rejection, and that a
SECURITY line is logged. profile_override_branch_is_absent_in_release asserts
the builder returns the generated deny-default profile rather than the caller's
string, and that the request policy is reflected in it.
profile_override_takes_precedence assert the dev behavior.
in debug, 555 + 36 in release, 0 failed.
-D warnings both clean; codegen gates green.
🔗 References
Stack, merge bottom-up. This PR targets
user/gudge/trust-model-relaxation-logging, so the diff shown is only this change; review #726 first.seatbelt.profileOverridein shipped builds