Skip to content

Make seatbelt.profileOverride dev-only (rejected by shipped builds) - #727

Open
Gudge (MGudgin) wants to merge 4 commits into
user/gudge/trust-model-relaxation-loggingfrom
user/gudge/trust-model-profileoverride-devonly
Open

Make seatbelt.profileOverride dev-only (rejected by shipped builds)#727
Gudge (MGudgin) wants to merge 4 commits into
user/gudge/trust-model-relaxation-loggingfrom
user/gudge/trust-model-profileoverride-devonly

Conversation

@MGudgin

@MGudgin Gudge (MGudgin) commented Aug 1, 2026

Copy link
Copy Markdown
Member

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.

🔗 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.

  1. Log boundary relaxations with secure-default warnings #726 — boundary-relaxation logging
  2. This PR — reject seatbelt.profileOverride in shipped builds
  3. Document the trust model and the outer-clamp design #728 — document the trust model and outer-clamp design

@MGudgin
Gudge (MGudgin) requested a review from a team as a code owner August 1, 2026 00:37
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Comment thread docs/macos-support/seatbelt-backend.md Outdated

@bbonaby Branden Bonaby (bbonaby) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MGudgin
Gudge (MGudgin) dismissed Branden Bonaby (bbonaby)’s stale review August 4, 2026 00:55

The merge-base changed after approval.

@bbonaby

Copy link
Copy Markdown
Collaborator

Gudge (@MGudgin), just have a merge conflict with this one.

@MGudgin
Gudge (MGudgin) requested a review from a team August 8, 2026 20:23
Copilot AI balanced review requested due to automatic review settings August 8, 2026 21:05
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/trust-model-profileoverride-devonly branch from 8d97f3f to d3ac5f7 Compare August 8, 2026 21:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restricts Seatbelt profile overrides to debug builds, preventing shipped binaries from honoring caller-supplied profiles.

Changes:

  • Rejects overrides during release configuration parsing.
  • Compiles override handling out of release profile generation.
  • Updates tests, schemas, generated types, and documentation.
Show a summary per file
File Description
src/core/wxc_common/src/wire.rs Documents the release restriction.
src/core/wxc_common/src/config_parser.rs Rejects overrides in release builds.
src/backends/seatbelt/common/src/profile_builder.rs Disables override generation in releases.
sdk/node/src/generated/wire.ts Updates generated field documentation.
schemas/dev/mxc-config.schema.0.8.0-dev.json Updates generated schema description.
docs/schema.md Documents the configuration restriction.
docs/macos-support/seatbelt-backend.md Documents dev-only override behavior.

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: 6/7 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread src/backends/seatbelt/common/src/profile_builder.rs Outdated
Comment thread src/core/wxc_common/src/config_parser.rs
Comment thread docs/schema.md Outdated
Comment thread docs/macos-support/seatbelt-backend.md Outdated
Copilot AI review requested due to automatic review settings August 8, 2026 21:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (4)

src/backends/seatbelt/common/src/profile_builder.rs:41

  • The parser does not strip this field in release builds; it rejects the entire request. Saying “strips” contradicts both make_seatbelt_config and the fail-closed behavior documented immediately above.
/// parser strips `profileOverride` in release anyway), so a shipped binary

sdk/node/tests/integration/macos-seatbelt.test.ts:251

  • This integration test is not actually restricted to release binaries. findSeatbeltExecutable() can select the SDK-bundled or Cargo debug executable, and build-mac.sh --debug copies that debug binary into the preferred SDK path. In that supported debug workflow the override is intentionally honored, so this assertion fails. Gate the test on a known release artifact (or make the expected result profile-aware) and retain a debug integration assertion for the intended pass-through behavior.
    const result = await spawnFromConfigAsync(config, seatbeltSpawnOptions);
    assert.notStrictEqual(result.exitCode, 0, 'release builds must reject profileOverride');

docs/schema.md:97

  • This updated example still nests seatbelt inside experimental, but the parser accepts Seatbelt configuration only at the top level and explicitly rejects experimental.seatbelt (config_parser.rs:1574-1579). A debug-build user following this example therefore cannot use the dev-only override at all. Move the whole seatbelt block alongside lxc rather than leaving it under experimental.
            "profileOverride": null,       // Dev-only escape hatch (rejected by release builds)

schemas/dev/mxc-config.schema.0.8.0-dev.json:809

  • The PR description says schema.md will document that released schemas still advertise this field as generally usable, but this change only updates the dev-schema description. The immutable 0.7 stable schema still calls it an optional override even though release binaries now reject it, so users validating against that schema need the promised compatibility note in docs/schema.md.
          "description": "Replace the generated profile entirely (dev-only escape hatch; rejected by release builds).",
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI review requested due to automatic review settings August 10, 2026 17:49
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/trust-model-profileoverride-devonly branch from e7772e8 to eea4d9d Compare August 10, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

src/core/wxc_common/src/wire.rs:554

  • This change is unrelated to the stated seatbelt.profileOverride hardening and removes/replaces a public IsolationSession wire contract (sizing/user start payloads, appId, sandbox IDs, required network acknowledgement, and corresponding SDK behavior). GitHub currently reports 109 changed files and five commits for this PR despite the description saying the diff contains only the Seatbelt change. Please rebase onto the intended #726 head or split the IsolationSession migration into its own PR so this security fix can be reviewed and merged independently.
  • Files reviewed: 104/109 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/backends/isolation_session/common/src/pipe_relay.rs
Copilot AI review requested due to automatic review settings August 10, 2026 18:00
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/trust-model-profileoverride-devonly branch from eea4d9d to fda8a6d Compare August 10, 2026 18:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (2)

src/backends/seatbelt/common/src/profile_builder.rs:41

  • The parser does not strip this field in release builds; it returns ConfigParse and rejects the request. Please keep this API documentation aligned with the fail-closed behavior so direct callers do not expect silent sanitization.
/// parser rejects configs that set `profileOverride` in release anyway), so a

docs/schema.md:97

  • The PR description says this document records the immutable stable-schema discrepancy, but this update does not mention it. Production users are directed above to stable 0.7, whose description still presents profileOverride as an ordinary optional override. Add an explicit compatibility note that the immutable schema still permits/describes the field while release binaries reject it at runtime.
            "storagePath": "C:\\wslc-storage",  // Image store path
  • Files reviewed: 8/9 changed files
  • Comments generated: 1
  • Review effort level: Balanced


it('should apply profile override from seatbelt config', { timeout: 30_000 }, async () => {
// Build a config with a custom seatbelt profile that allows everything
it('should reject profile override in release builds', { timeout: 30_000 }, async () => {
Copilot AI review requested due to automatic review settings August 10, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI review requested due to automatic review settings August 10, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 12/13 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines +59 to +60
run: printf 'release\n' >
src/target/aarch64-apple-darwin/release/mxc-exec-mac.build-type
Gudge and others added 4 commits August 10, 2026 13:18
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
This PR fixes the release-facing documentation and integration coverage for the dev-only Seatbelt profile override.

Details

* Describe release behavior consistently as rejecting profileOverride rather than silently stripping it.
* Document the restriction on the public TypeScript SeatbeltConfig type.
* Make the packaged macOS integration suite assert release rejection instead of expecting the debug-only override path.

Tests

* npm run build (sdk/node)
* npm run build (sdk/node/tests/integration)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9ded049-d001-46d4-b6c2-9351f4ce1468
This PR clarifies how release builds handle the dev-only Seatbelt profile override.

Details

* State that the release parser rejects the whole config rather than stripping the field.
* Explain that immutable stable schemas may describe fields that current release builds reject at runtime.

Tests

* cargo fmt --all -- --check
* cargo check -p seatbelt_common
* git diff --check

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9ded049-d001-46d4-b6c2-9351f4ce1468
This PR makes the macOS profileOverride integration coverage follow the actual executor build profile.

Details

* Persist a debug or release marker beside packaged mxc-exec-mac binaries in local and CI builds.
* Detect the marker in the integration harness, with an explicit override for manually supplied executables.
* Split profileOverride coverage so release builds assert rejection and debug builds assert override precedence.
* Verify the marker is present and valid in packaged macOS SDKs.

Tests

* bash -n build-mac.sh
* Parsed .github/workflows/Build.MacOS.Job.yml with PyYAML
* npm run build (sdk/node)
* npm run build (sdk/node/tests/integration)
* cargo test -p wxc_common -p seatbelt_common: 632 passed
* cargo test --release -p wxc_common -p seatbelt_common: 632 passed
* Build-type environment override and package-marker probes passed
* macOS runtime integration was not runnable on this Windows host

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9ded049-d001-46d4-b6c2-9351f4ce1468
Copilot AI review requested due to automatic review settings August 10, 2026 20:20
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/trust-model-profileoverride-devonly branch from c24937f to b2931b6 Compare August 10, 2026 20:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 12/13 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines +102 to +105
export const EXPECTED_MACOS_PACKAGE_FILES = [
...EXPECTED_MACOS_BINARIES,
SEATBELT_BUILD_TYPE_FILENAME,
];
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants