From a1059915a3923731f34f66e0c311a87c95a4721b Mon Sep 17 00:00:00 2001 From: Alex Shabalin <110031243+alex-sparus@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:03:39 +0200 Subject: [PATCH] fix(env): stop shipping the development profile A Vite define is text substitution, so passing an object emitted an object literal where a string was expected; JSON.parse threw and the catch silently loaded env.development.json. Every release therefore ran its frontend with VITE_NIXMAC_SKIP_PERMISSIONS on, disabling the permissions gate. Confirmed against a CI production bundle. - one object-valued define, validated and coerced at build time, and no fallback: a bad profile fails the build, or throws at startup - NIXMAC_ENV only selects a profile and no longer overwrites the selected file's own value; unknown selectors fail the build instead of falling through to development - a prod profile refuses the skip-permissions and nix-installed bypasses whatever the profile asks for - Rust resolves the environment through one path instead of three - prerequisite steps are gated on onboarding being unfinished, so a missing permission after setup is a banner, not an internal error - an inconclusive Full Disk Access probe no longer counts against the required-permission gate --- .cursor/rules/native-env.mdc | 2 +- .github/workflows/build.yaml | 42 ++++++- apps/native/nixmac-profile.ts | 77 +++++++++--- apps/native/src-tauri/build.rs | 44 +++++-- .../configurable-derive/src/codegen.rs | 20 ++- .../configurable-derive/src/fields.rs | 4 +- .../resources/schemas/env.schema.json | 2 +- apps/native/src-tauri/src/env/config.rs | 6 +- apps/native/src-tauri/src/env/sources.rs | 9 +- apps/native/src-tauri/src/env_keys.rs | 10 +- apps/native/src-tauri/src/main.rs | 15 +-- .../src-tauri/src/system/permissions.rs | 118 +++++++++++++++--- .../src/components/widget/layout/header.tsx | 1 + .../src/components/widget/utils.test.ts | 27 +++- apps/native/src/components/widget/utils.ts | 14 ++- apps/native/src/hooks/use-current-step.ts | 4 + apps/native/src/lib/env-profile-schema.ts | 51 ++++++++ apps/native/src/lib/env.test.ts | 54 ++++++++ apps/native/src/lib/env.ts | 116 ++++++----------- apps/native/src/vite-env.d.ts | 8 +- nix/dev.nix | 8 +- scripts/env.sh | 4 +- 22 files changed, 459 insertions(+), 177 deletions(-) create mode 100644 apps/native/src/lib/env-profile-schema.ts create mode 100644 apps/native/src/lib/env.test.ts diff --git a/.cursor/rules/native-env.mdc b/.cursor/rules/native-env.mdc index 5300e8175..f7f297aa5 100644 --- a/.cursor/rules/native-env.mdc +++ b/.cursor/rules/native-env.mdc @@ -8,7 +8,7 @@ alwaysApply: false **Do not use `process.env` or `import.meta.env` anywhere except `apps/native/src/lib/env.ts`.** -All app code reads deployment settings through exports from that module (`settings`, `nixmacEnvironment`, `getProfileValue`, etc.). +All app code reads deployment settings through exports from that module (`settings`, `nixmacEnvironment`, `nixmacVersion`, `isE2eProfile`). Benefits: single validation path, typed profile JSON, no scattered env reads, and build-time profile baking stays consistent with Rust (`build.rs`). diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 75ab3a674..66ade64a0 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -169,7 +169,7 @@ jobs: - name: Cargo clippy run: cargo clippy --workspace --all-targets --features nixmac/codegen -- -D warnings env: - NIXMAC_ENV: prod + NIXMAC_ENV: production # Decide what kind of build this is: # - tag: push of refs/tags/v* → ship that exact version on stable @@ -251,11 +251,49 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Pass DSNs and build metadata into the tauri action step so build.rs can read them SENTRY_DSN: ${{ secrets.SENTRY_DSN }} - NIXMAC_ENV: prod + NIXMAC_ENV: production NIXMAC_VERSION: ${{ steps.sync-version.outputs.build_version }} VITE_SERVER_URL: ${{ secrets.VITE_SERVER_URL }} SUBMITTED_FEEDBACK_DSN: ${{ secrets.SUBMITTED_FEEDBACK_DSN }} + # An unset NIXMAC_ENV selects the development profile, which turns the + # permissions gate off. Check the artifacts themselves name production, + # rather than trusting that the env above reached both bakers: the + # frontend bundle carries its profile as a Vite define, the binary as a + # build.rs embed, and they are baked by separate code paths. + # + # esbuild may drop the quotes on the object key, so both greps accept the + # key quoted or bare. The second one is the important half: a release + # bundle that carries a non-production profile at all has a path back to + # one, whether or not it also carries the right one. + - name: Verify artifacts carry the production profile + run: | + set -euo pipefail + + if [ ! -d apps/native/dist ]; then + echo "ERROR: apps/native/dist is missing; the frontend was not built" + exit 1 + fi + if ! grep -rqE '"?NIXMAC_ENV"?:"production"' apps/native/dist; then + echo "ERROR: frontend bundle in apps/native/dist does not carry the production profile" + exit 1 + fi + if grep -rqE '"?NIXMAC_ENV"?:"(development|e2e)"' apps/native/dist; then + echo "ERROR: frontend bundle in apps/native/dist also carries a non-production profile" + grep -rhoE '"?NIXMAC_ENV"?:"(development|e2e)"' apps/native/dist | sort -u + exit 1 + fi + + APP_PATH=$(find target/release/bundle/macos -name "*.app" -type d 2>/dev/null | sed -n '1p' || true) + if [ -z "$APP_PATH" ]; then + echo "ERROR: no .app bundle found under target/release/bundle/macos" + exit 1 + fi + if ! grep -qa '"NIXMAC_ENV":"production"' "$APP_PATH/Contents/MacOS/nixmac"; then + echo "ERROR: $APP_PATH does not embed the production profile" + exit 1 + fi + - name: Unit Test frontend working-directory: apps/native run: bun run test:unit diff --git a/apps/native/nixmac-profile.ts b/apps/native/nixmac-profile.ts index 6c18a2d2c..204b8114c 100644 --- a/apps/native/nixmac-profile.ts +++ b/apps/native/nixmac-profile.ts @@ -1,5 +1,6 @@ import { readFileSync } from "node:fs"; import path from "node:path"; +import { EnvProfileSchema, NIXMAC_ENVS, type NixmacEnv } from "./src/lib/env-profile-schema"; type NixmacProfileName = "development" | "release" | "e2e"; @@ -8,16 +9,28 @@ function readProfileJson(nativeAppDir: string, name: NixmacProfileName): Record< return JSON.parse(raw) as Record; } -/** Profile file selection — keep in sync with `apps/native/src-tauri/build.rs`. */ -function resolveNixmacProfile(): NixmacProfileName { - switch (process.env.NIXMAC_ENV ?? "development") { - case "prod": +/** + * Profile file selection — keep in sync with `apps/native/src-tauri/build.rs`. + * + * Unset means development. Any other value is a mistake in the build command, + * not a request for the default. + * + * Returns the selector as well as the file, so the caller can check that the + * file it picked names the same environment. + */ +function resolveNixmacProfile(): { selector: NixmacEnv; file: NixmacProfileName } { + const selector = process.env.NIXMAC_ENV ?? "development"; + switch (selector) { + case "development": + return { selector, file: "development" }; case "production": - return "release"; + return { selector, file: "release" }; case "e2e": - return "e2e"; + return { selector, file: "e2e" }; default: - return "development"; + throw new Error( + `NIXMAC_ENV must be unset or one of ${NIXMAC_ENVS.join(", ")}; got ${JSON.stringify(selector)}`, + ); } } @@ -65,6 +78,15 @@ const OVERRIDABLE_PREFIXES = [ "NIX_INSTALLED_", ] as const; +/** + * Keys process env must never overwrite. + * + * `NIXMAC_ENV` picks which profile file to read, and each profile names the same + * environment in its own `NIXMAC_ENV` key. A selector is not a setting: letting + * process env write that key too would let it drift from the file it selected. + */ +const NON_OVERRIDABLE_KEYS = new Set(["$schema", "NIXMAC_ENV"]); + function isOverridableKey(key: string): boolean { return OVERRIDABLE_PREFIXES.some((prefix) => key.startsWith(prefix)); } @@ -80,7 +102,8 @@ function mergeProfileWithProcessEnv( const merged: Record = { ...base }; for (const [key, envValue] of Object.entries(process.env)) { - if (key === "$schema" || envValue === undefined || envValue.trim() === "") continue; + if (NON_OVERRIDABLE_KEYS.has(key) || envValue === undefined || envValue.trim() === "") + continue; if (!(key in merged) && !isOverridableKey(key)) continue; merged[key] = coerceEnvOverride(merged[key], envValue); } @@ -89,23 +112,37 @@ function mergeProfileWithProcessEnv( return merged; } -function loadCommittedProfile( +function resolveMergedProfile( nativeAppDir: string, - name: NixmacProfileName, + file: NixmacProfileName, ): Record { - return readProfileJson(nativeAppDir, name); -} - -function resolveMergedProfile(nativeAppDir: string): Record { - const base = loadCommittedProfile(nativeAppDir, resolveNixmacProfile()); - return mergeProfileWithProcessEnv(base, nativeAppDir); + return mergeProfileWithProcessEnv(readProfileJson(nativeAppDir, file), nativeAppDir); } +/** + * Vite `define` entries that bake the selected profile into the bundle. + * + * A define is raw text substitution, so this emits a JavaScript object literal + * that `src/lib/env.ts` consumes directly: no string to parse, and therefore no + * parse for a bad profile to fall back from. Validating and coercing here means + * an invalid profile fails the build instead of the app. + * + * The schema only checks that `NIXMAC_ENV` is one of the three names, so the + * file could name an environment other than the one the selector asked for. + * `mayBypassUserGates` in `src/lib/env.ts` reads that name to decide whether the + * skip-permissions and nix-installed bypasses are allowed at all, so a profile + * mislabelled `development` would switch that lockout off in a release build. + * Checking the two agree is what keeps the name honest. + */ export function nixmacBuildDefines(nativeAppDir: string): Record { - const profileName = resolveNixmacProfile(); - const merged = resolveMergedProfile(nativeAppDir); + const { selector, file } = resolveNixmacProfile(); + const profile = EnvProfileSchema.parse(resolveMergedProfile(nativeAppDir, file)); + if (profile.NIXMAC_ENV !== selector) { + throw new Error( + `env.${file}.json declares NIXMAC_ENV ${JSON.stringify(profile.NIXMAC_ENV)}, but this build selected ${JSON.stringify(selector)}; the two must name the same environment`, + ); + } return { - __NIXMAC_PROFILE__: JSON.stringify(profileName), - __NIXMAC_PROFILE_JSON__: JSON.stringify(merged), + __NIXMAC_PROFILE_DATA__: JSON.stringify(profile), }; } diff --git a/apps/native/src-tauri/build.rs b/apps/native/src-tauri/build.rs index 9af6ee637..1b7d65788 100644 --- a/apps/native/src-tauri/build.rs +++ b/apps/native/src-tauri/build.rs @@ -7,13 +7,20 @@ use std::path::Path; use std::process::Command; /// Embed `apps/native/env.{development,release,e2e}.json` selected by `NIXMAC_ENV`. +/// +/// Accepted values must stay in sync with `apps/native/nixmac-profile.ts`. +/// Unset means development; anything else stops the build rather than falling +/// through to the development profile. fn embed_build_profile() { let native_app_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join(".."); - let profile = std::env::var("NIXMAC_ENV").unwrap_or_else(|_| "development".to_string()); - let file = match profile.as_str() { - "prod" | "production" => "env.release.json", + let selector = std::env::var("NIXMAC_ENV").unwrap_or_else(|_| "development".to_string()); + let file = match selector.as_str() { + "development" => "env.development.json", + "production" => "env.release.json", "e2e" => "env.e2e.json", - _ => "env.development.json", + other => { + panic!("NIXMAC_ENV must be unset or one of development, production, e2e; got {other:?}") + } }; let path = native_app_dir.join(file); @@ -25,11 +32,30 @@ fn embed_build_profile() { ); } - let json = std::fs::read_to_string(&path).unwrap_or_else(|_| "{}".to_string()); - let minified = serde_json::from_str::(&json) - .ok() - .and_then(|value| serde_json::to_string(&value).ok()) - .unwrap_or_else(|| "{}".to_string()); + // An unreadable or malformed profile stops the build. Degrading to "{}" + // compiles an app whose every setting silently falls back to its default. + let json = std::fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("cannot read {}: {error}", path.display())); + let value: serde_json::Value = serde_json::from_str(&json) + .unwrap_or_else(|error| panic!("cannot parse {}: {error}", path.display())); + // The selector picks the file; the file names the same environment in its own + // `NIXMAC_ENV` key. That key is what `crate::env::nixmac_env()` reports for + // telemetry and the startup log, unless `NIXMAC_ENV` is set in the running + // process, which takes precedence over the embed. A profile mislabelled + // `development` would therefore have a release build report itself as a + // development one. The frontend is baked from its own copy of the profile + // and makes the same check there (`apps/native/nixmac-profile.ts`). + let declared = value.get("NIXMAC_ENV"); + if declared.and_then(serde_json::Value::as_str) != Some(selector.as_str()) { + panic!( + "{} declares NIXMAC_ENV {}, but this build selected {selector:?}; the two must name the same environment", + path.display(), + declared.map_or_else(|| "no value at all".to_string(), ToString::to_string) + ); + } + + let minified = serde_json::to_string(&value) + .unwrap_or_else(|error| panic!("cannot re-encode {}: {error}", path.display())); println!("cargo:rustc-env=NIXMAC_ENV_PROFILE_JSON={minified}"); } diff --git a/apps/native/src-tauri/configurable-derive/src/codegen.rs b/apps/native/src-tauri/configurable-derive/src/codegen.rs index 2b5ebcbbb..21ab1ca17 100644 --- a/apps/native/src-tauri/configurable-derive/src/codegen.rs +++ b/apps/native/src-tauri/configurable-derive/src/codegen.rs @@ -191,7 +191,7 @@ fn build_scope_methods( } fn __resolve_string( - profile: Option<&serde_json::Value>, + profile: &serde_json::Value, key: &str, env_var: &str, build_embed: bool, @@ -205,19 +205,17 @@ fn build_scope_methods( return value; } } - if let Some(profile) = profile { - if let Some(value) = profile.get(key).and_then(|value| value.as_str()) { - let value = value.trim(); - if !value.is_empty() { - return value.to_string(); - } + if let Some(value) = profile.get(key).and_then(|value| value.as_str()) { + let value = value.trim(); + if !value.is_empty() { + return value.to_string(); } } default.to_string() } fn __resolve_bool( - profile: Option<&serde_json::Value>, + profile: &serde_json::Value, key: &str, env_var: &str, default: bool, @@ -225,10 +223,8 @@ fn build_scope_methods( if let Some(value) = crate::env::sources::trimmed_env(env_var) { return crate::env::sources::env_is_truthy(&value); } - if let Some(profile) = profile { - if let Some(value) = profile.get(key).and_then(|value| value.as_bool()) { - return value; - } + if let Some(value) = profile.get(key).and_then(|value| value.as_bool()) { + return value; } default } diff --git a/apps/native/src-tauri/configurable-derive/src/fields.rs b/apps/native/src-tauri/configurable-derive/src/fields.rs index a98c783f0..7ed936191 100644 --- a/apps/native/src-tauri/configurable-derive/src/fields.rs +++ b/apps/native/src-tauri/configurable-derive/src/fields.rs @@ -127,7 +127,7 @@ fn generate_field(field: &syn::Field, scope: StoreScope) -> syn::Result quote! { #ident: Self::__resolve_bool( - __build_profile.as_ref(), + &__build_profile, #profile_key_lit, #env_var_lit, #default, @@ -135,7 +135,7 @@ fn generate_field(field: &syn::Field, scope: StoreScope) -> syn::Result quote! { #ident: Self::__resolve_string( - __build_profile.as_ref(), + &__build_profile, #profile_key_lit, #env_var_lit, #build_embed, diff --git a/apps/native/src-tauri/resources/schemas/env.schema.json b/apps/native/src-tauri/resources/schemas/env.schema.json index 8f8c4609d..b3bcd0e82 100644 --- a/apps/native/src-tauri/resources/schemas/env.schema.json +++ b/apps/native/src-tauri/resources/schemas/env.schema.json @@ -32,7 +32,7 @@ "type": "boolean" }, "NIXMAC_ENV": { - "default": "prod", + "default": "production", "title": "Deployment environment", "type": "string" }, diff --git a/apps/native/src-tauri/src/env/config.rs b/apps/native/src-tauri/src/env/config.rs index bc1c7c489..134a81b14 100644 --- a/apps/native/src-tauri/src/env/config.rs +++ b/apps/native/src-tauri/src/env/config.rs @@ -52,9 +52,11 @@ pub struct NixmacEnvSettings { )] pub sentry_dsn: String, + /// No `build_embed`: this is the selector for which profile gets embedded, so + /// it resolves from the process environment, then from that profile's own + /// `NIXMAC_ENV` key — never from a separate embedded copy of the selector. #[config( - default = "prod", - build_embed = true, + default = "production", env_var = "NIXMAC_ENV", label = "Deployment environment" )] diff --git a/apps/native/src-tauri/src/env/sources.rs b/apps/native/src-tauri/src/env/sources.rs index 6e845f99c..7ee3865f3 100644 --- a/apps/native/src-tauri/src/env/sources.rs +++ b/apps/native/src-tauri/src/env/sources.rs @@ -16,7 +16,6 @@ pub fn build_embed(name: &str) -> Option { "SENTRY_DSN" => option_env!("SENTRY_DSN").map(str::to_string), "VITE_SERVER_URL" => option_env!("VITE_SERVER_URL").map(str::to_string), "SUBMITTED_FEEDBACK_DSN" => option_env!("SUBMITTED_FEEDBACK_DSN").map(str::to_string), - "NIXMAC_ENV" => option_env!("NIXMAC_ENV").map(str::to_string), "NIXMAC_VERSION" => option_env!("NIXMAC_VERSION").map(str::to_string), _ => None, }; @@ -25,8 +24,9 @@ pub fn build_embed(name: &str) -> Option { } /// JSON profile from `apps/native/env.{development,release,e2e}.json`, embedded at compile time. -pub fn build_profile() -> Option { - option_env!("NIXMAC_ENV_PROFILE_JSON").and_then(|raw| serde_json::from_str(raw).ok()) +pub fn build_profile() -> serde_json::Value { + serde_json::from_str(env!("NIXMAC_ENV_PROFILE_JSON")) + .expect("NIXMAC_ENV_PROFILE_JSON is minified by build.rs and must parse") } #[cfg(test)] @@ -35,8 +35,7 @@ mod tests { #[test] fn embedded_build_profile_parses_and_uses_env_var_keys() { - let profile = - build_profile().expect("NIXMAC_ENV_PROFILE_JSON should parse at compile time"); + let profile = build_profile(); assert_eq!( profile .get("VITE_SERVER_URL") diff --git a/apps/native/src-tauri/src/env_keys.rs b/apps/native/src-tauri/src/env_keys.rs index 2196ef87b..d6acf4852 100644 --- a/apps/native/src-tauri/src/env_keys.rs +++ b/apps/native/src-tauri/src/env_keys.rs @@ -4,12 +4,10 @@ #[allow(dead_code)] /// Environment variables embedded at build time via `build.rs` (`cargo:rustc-env`). -pub const BUILD_EMBED_KEYS: &[&str] = &[ - "SENTRY_DSN", - "VITE_SERVER_URL", - "SUBMITTED_FEEDBACK_DSN", - "NIXMAC_ENV", -]; +/// `NIXMAC_ENV` is deliberately absent: it selects which profile to embed, so +/// embedding it as a value too let the selector overwrite the selected file's +/// own `NIXMAC_ENV`. The value comes from the embedded profile JSON instead. +pub const BUILD_EMBED_KEYS: &[&str] = &["SENTRY_DSN", "VITE_SERVER_URL", "SUBMITTED_FEEDBACK_DSN"]; /// Application environment variable names. #[allow(dead_code)] diff --git a/apps/native/src-tauri/src/main.rs b/apps/native/src-tauri/src/main.rs index 3eed3a65f..c6b455920 100644 --- a/apps/native/src-tauri/src/main.rs +++ b/apps/native/src-tauri/src/main.rs @@ -502,17 +502,10 @@ fn run_gui_mode( std::sync::Mutex>, >, ) { - // Prefer compile-time embedded vars (set by build.rs via `cargo:rustc-env`), - // fall back to runtime environment variables. - let nixmac_env = option_env!("NIXMAC_ENV") - .map(|s| s.to_string()) - .or_else(|| std::env::var("NIXMAC_ENV").ok()) - .unwrap_or_else(|| "prod".to_string()); - - let nixmac_version = option_env!("NIXMAC_VERSION") - .map(|s| s.to_string()) - .or_else(|| std::env::var("NIXMAC_VERSION").ok()) - .unwrap_or_else(|| "unknown".to_string()); + // Both read through `crate::env`, which owns the precedence, so the log line + // below reports the same values telemetry does. + let nixmac_env = crate::env::nixmac_env(); + let nixmac_version = crate::env::nixmac_version(); let mut builder = tauri::Builder::default().plugin(tauri_plugin_http::init()); let orpc_router = orpc::build_router(); diff --git a/apps/native/src-tauri/src/system/permissions.rs b/apps/native/src-tauri/src/system/permissions.rs index cd98e8a4a..8c0cef7ab 100644 --- a/apps/native/src-tauri/src/system/permissions.rs +++ b/apps/native/src-tauri/src/system/permissions.rs @@ -238,28 +238,70 @@ fn check_folder_access(path: &PathBuf) -> PermissionStatus { } } -/// Check if we have Full Disk Access. +/// Opens every inconclusive Full Disk Access message: the check reached no +/// verdict, which is not the same as finding the access missing. +const FULL_DISK_ACCESS_INCONCLUSIVE_PREFIX: &str = "Could not determine Full Disk Access: "; + +/// Closes every inconclusive Full Disk Access message with the one action a +/// user can take when the check cannot decide for them. +const FULL_DISK_ACCESS_INCONCLUSIVE_HINT: &str = " If a rebuild fails with a permissions error, add nixmac under System Settings → Privacy & Security → Full Disk Access."; + +/// Why the probe reached no verdict: every path it reads was missing. +/// +/// The probe reads files a granted process can read and a denied one cannot. +/// If none of them exist there is nothing to read, so it proves neither state — +/// say so, rather than implying we checked and found the access missing. +const FULL_DISK_ACCESS_NO_PROBE_PATHS: &str = + "none of the files this check probes exist on this Mac."; + +/// Why the probe reached no verdict: the Mac reported no home directory. +/// +/// Every probe path but one is under the home directory, so without it the +/// check never ran — again inconclusive rather than denied. +const FULL_DISK_ACCESS_NO_HOME: &str = + "this Mac reported no home directory, so the check could not run."; + +/// Shown on the Full Disk Access row when the probe reached no verdict. +/// +/// Both inconclusive paths say the same thing either side of their own reason, +/// so the wording stays in one place. Built only on those paths, never on the +/// granted or denied ones. +fn full_disk_access_inconclusive(reason: &str) -> String { + format!("{FULL_DISK_ACCESS_INCONCLUSIVE_PREFIX}{reason}{FULL_DISK_ACCESS_INCONCLUSIVE_HINT}") +} + +/// Check if we have Full Disk Access, with an explanation when inconclusive. /// /// Probes several TCC-gated paths. A successful read on any one is proof of /// FDA. A PermissionDenied on any one is proof of the opposite — even if the /// user has nixmac listed and toggled on in System Settings, a stale TCC /// entry (e.g. codesign requirement mismatch after an update-in-place) can /// leave the grant silently inactive, and reads will fail with -/// PermissionDenied. Only if every probe path is missing (NotFound) do we -/// fall back to Pending. -fn check_full_disk_access() -> PermissionStatus { +/// PermissionDenied. If every probe path is missing (NotFound) the probe has +/// established nothing, which is `Unknown` rather than `Pending` — the latter +/// reads as "not granted yet", which is a claim this probe cannot make. +/// +/// `Unknown` still fails the onboarding gate on its own; what keeps an +/// unverifiable result from holding the gate shut is the caller dropping the +/// row's `required` flag (see the `full-disk` arm of `check_all_permissions`). +fn check_full_disk_access() -> (PermissionStatus, Option) { if vite_skip_permissions_enabled() { debug!("VITE_NIXMAC_SKIP_PERMISSIONS is set, assuming Full Disk Access granted"); - return PermissionStatus::Granted; + return (PermissionStatus::Granted, None); } if e2e_skip_permissions_enabled() { debug!("E2E permission skip enabled, assuming Full Disk Access granted"); - return PermissionStatus::Granted; + return (PermissionStatus::Granted, None); } let home = match dirs::home_dir() { Some(h) => h, - None => return PermissionStatus::Unknown, + None => { + return ( + PermissionStatus::Unknown, + Some(full_disk_access_inconclusive(FULL_DISK_ACCESS_NO_HOME)), + ); + } }; // (path, is_dir). Ordered by how reliably the path exists on a typical @@ -286,7 +328,7 @@ fn check_full_disk_access() -> PermissionStatus { match result { Ok(_) => { debug!("Full Disk Access granted (probe succeeded: {:?})", path); - return PermissionStatus::Granted; + return (PermissionStatus::Granted, None); } Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { debug!("Full Disk Access denied (probe blocked: {:?})", path); @@ -297,10 +339,15 @@ fn check_full_disk_access() -> PermissionStatus { } if saw_denied { - PermissionStatus::Denied + (PermissionStatus::Denied, None) } else { debug!("Full Disk Access check inconclusive — no probe path existed"); - PermissionStatus::Pending + ( + PermissionStatus::Unknown, + Some(full_disk_access_inconclusive( + FULL_DISK_ACCESS_NO_PROBE_PATHS, + )), + ) } } @@ -345,7 +392,24 @@ pub fn check_all_permissions() -> PermissionsState { "desktop" => check_desktop_access(), "documents" => check_documents_access(), "admin" => check_admin_privileges(), - "full-disk" => check_full_disk_access(), + "full-disk" => { + let (status, detail) = check_full_disk_access(); + // Keep the default "how to grant it" text unless the probe has + // something more accurate to say. + if let Some(detail) = detail { + perm.instructions = Some(detail); + } + // A probe that could not decide must not keep the gate shut — + // the same reasoning that makes app-management Recommended + // rather than Required (see `app_management_permission`), except + // discovered at runtime. Requiring access we cannot verify shows + // a permissions banner to users who may well have granted it. + // Only this row relaxes: every other `Unknown` still blocks. + if status == PermissionStatus::Unknown { + perm.required = false; + } + status + } "app-management" => check_app_management(), "privileged-helper" => { let (status, detail) = check_privileged_helper(); @@ -356,7 +420,10 @@ pub fn check_all_permissions() -> PermissionsState { } status } - _ => PermissionStatus::Unknown, + other => { + debug_assert!(false, "no probe for permission id {other:?}"); + PermissionStatus::Unknown + } }; // If this permission is required and not granted, mark all_required_granted as false @@ -501,17 +568,18 @@ pub fn request_permission(permission_id: &str) -> Result { .spawn(); // Re-check the status + let (status, detail) = check_full_disk_access(); Ok(Permission { id: "full-disk".to_string(), name: "Full Disk Access".to_string(), description: "Required for darwin-rebuild to apply system changes".to_string(), required: true, can_request_programmatically: false, - status: check_full_disk_access(), - instructions: Some( + status, + instructions: Some(detail.unwrap_or_else(|| { "First make sure nixmac is in your Applications folder (not running from the install disk image). Then go to System Settings → Privacy & Security → Full Disk Access and add nixmac to the list." - .to_string(), - ), + .to_string() + })), }) } "app-management" => { @@ -805,6 +873,24 @@ mod tests { ); } + #[test] + fn inconclusive_full_disk_access_is_unknown_and_explains_itself() { + // Only meaningful when the probe is actually inconclusive; on a Mac + // with Safari or Mail the probe reaches a verdict and this is skipped. + // Either inconclusive path must explain itself — which one ran depends + // on the machine, so assert the property rather than the wording. + let (status, detail) = check_full_disk_access(); + if status == PermissionStatus::Unknown { + let detail = detail.expect("an inconclusive probe must explain itself"); + assert!(detail.starts_with(FULL_DISK_ACCESS_INCONCLUSIVE_PREFIX)); + } else { + assert!(matches!( + status, + PermissionStatus::Granted | PermissionStatus::Denied + )); + } + } + #[test] fn app_management_is_not_inferred_from_full_disk_access() { assert_eq!(check_app_management(), PermissionStatus::Pending); diff --git a/apps/native/src/components/widget/layout/header.tsx b/apps/native/src/components/widget/layout/header.tsx index c2f3ff5ca..9be1febee 100644 --- a/apps/native/src/components/widget/layout/header.tsx +++ b/apps/native/src/components/widget/layout/header.tsx @@ -36,6 +36,7 @@ export function Header() { activeStepOverride: state.activeStepOverride, hasChanges: (viewModel.git?.changes.length ?? 0) > 0, rebuildNeeded: viewModel.build.rebuildNeeded, + onboardingCompletedAt: viewModel.onboardingState?.completedAt ?? null, }); if (step !== "setup" && state.error && state.error !== prevState.error) { setIsPulsing(true); diff --git a/apps/native/src/components/widget/utils.test.ts b/apps/native/src/components/widget/utils.test.ts index 82837a870..4e9d3f522 100644 --- a/apps/native/src/components/widget/utils.test.ts +++ b/apps/native/src/components/widget/utils.test.ts @@ -215,6 +215,7 @@ describe("computeCurrentStep — diff gating", () => { activeStepOverride: null as EvolveStep | null, hasChanges: false, rebuildNeeded: false, + onboardingCompletedAt: 1_700_000_000, ...overrides, }; } @@ -276,7 +277,18 @@ describe("computeCurrentStep — diff gating", () => { const incomplete = { allRequiredGranted: false } as never; it("routes to permissions while a required permission is missing", () => { - expect(computeCurrentStep(readyState({ permissionsState: incomplete }))).toBe("permissions"); + expect( + computeCurrentStep( + readyState({ permissionsState: incomplete, onboardingCompletedAt: null }), + ), + ).toBe("permissions"); + }); + + it("stops routing to permissions once onboarding has completed", () => { + // After the wizard, a missing prerequisite is the repair banner's job. + // Returning "permissions" here reaches an assertion in widget.tsx that + // shows the user an internal-error banner instead. + expect(computeCurrentStep(readyState({ permissionsState: incomplete }))).toBe("begin"); }); it("honors settings.skipPermissions exactly like the onboarding gate", () => { @@ -290,11 +302,24 @@ describe("computeCurrentStep — diff gating", () => { permissionsState: incomplete, hasChanges: true, evolveState: evolveAt("manualEvolve"), + onboardingCompletedAt: null, }), ), ).toBe("manualEvolve"); }); }); + + describe("nix-setup gate", () => { + it("routes to nix-setup while onboarding and nix is missing", () => { + expect( + computeCurrentStep(readyState({ nixInstalled: false, onboardingCompletedAt: null })), + ).toBe("nix-setup"); + }); + + it("stops routing to nix-setup once onboarding has completed", () => { + expect(computeCurrentStep(readyState({ nixInstalled: false }))).toBe("begin"); + }); + }); }); describe("configRelativePath", () => { diff --git a/apps/native/src/components/widget/utils.ts b/apps/native/src/components/widget/utils.ts index 0167fe7ba..437bf82d8 100644 --- a/apps/native/src/components/widget/utils.ts +++ b/apps/native/src/components/widget/utils.ts @@ -27,6 +27,16 @@ type CurrentStepState = { hasChanges: boolean; /** Whether saved configuration is newer than the currently running system. */ rebuildNeeded: boolean; + /** + * When onboarding finished, or null while it is still running. + * + * Prerequisite steps belong to the wizard, which the backend completion latch + * decides to show (`onboarding/use-onboarding-flow.ts`). Once onboarding is + * done, a missing prerequisite is a banner rather than a step: `widget.tsx` + * has no case for "permissions" outside onboarding and treats reaching it as + * a programming error. + */ + onboardingCompletedAt: number | null; }; const orderByStep: { [key in EvolveStep]: number } = { @@ -40,6 +50,7 @@ const orderByStep: { [key in EvolveStep]: number } = { export function computeCurrentStep(state: CurrentStepState): WidgetStep { const hasConfigDir = !!state.configDir; const hasHost = !!state.host && state.hosts.includes(state.host); + const inOnboarding = state.onboardingCompletedAt === null; // Must mirror useOnboardingFlow's permissionsReady gate (including the // skipPermissions bypass): if the two disagree, OnboardingFlow considers // setup complete while this returns "permissions" — a step the widget has @@ -50,11 +61,12 @@ export function computeCurrentStep(state: CurrentStepState): WidgetStep { state.permissionsState && !state.permissionsState.allRequiredGranted; - if (permissionsCheckedAndIncomplete) { + if (inOnboarding && permissionsCheckedAndIncomplete) { return "permissions"; } if ( + inOnboarding && state.nixInstalled !== true && settings.nixInstalledOverride !== true // bypass used for testing ) { diff --git a/apps/native/src/hooks/use-current-step.ts b/apps/native/src/hooks/use-current-step.ts index eb8826123..88d6e211a 100644 --- a/apps/native/src/hooks/use-current-step.ts +++ b/apps/native/src/hooks/use-current-step.ts @@ -33,6 +33,9 @@ export function useCurrentStep(): WidgetStep { (state) => (state.git?.changes.length ?? 0) > 0, ); const rebuildNeeded = useViewModel((state) => state.build.rebuildNeeded); + const onboardingCompletedAt = useViewModel( + (state) => state.onboardingState?.completedAt ?? null, + ); return computeCurrentStep({ nixInstalled, darwinRebuildAvailable, @@ -49,5 +52,6 @@ export function useCurrentStep(): WidgetStep { activeStepOverride, hasChanges, rebuildNeeded, + onboardingCompletedAt, }); } diff --git a/apps/native/src/lib/env-profile-schema.ts b/apps/native/src/lib/env-profile-schema.ts new file mode 100644 index 000000000..68f4f51f5 --- /dev/null +++ b/apps/native/src/lib/env-profile-schema.ts @@ -0,0 +1,51 @@ +import { z } from "zod"; + +const envBool = z.preprocess((value) => { + if (typeof value === "boolean") return value; + if (typeof value === "string") { + const normalized = value.trim().toLowerCase(); + return normalized === "true" || normalized === "1" || normalized === "yes"; + } + return false; +}, z.boolean()); + +const optionalEnvString = z.preprocess((value) => { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +}, z.string().optional()); + +/** + * Deployment environment, named by the `NIXMAC_ENV` key of each committed + * profile. These are exactly the values `NIXMAC_ENV` may take when selecting a + * profile, and both selectors refuse a file whose own `NIXMAC_ENV` is not the + * value that selected it — see `nixmac-profile.ts` and `src-tauri/build.rs`. + */ +export const NIXMAC_ENVS = ["development", "production", "e2e"] as const; + +export type NixmacEnv = (typeof NIXMAC_ENVS)[number]; + +/** + * Shape of `apps/native/env.{development,release,e2e}.json` after the + * build-time merge with process env. + * + * Imported by the build script, which validates and coerces a profile before + * baking it into the bundle, and by `src/lib/env.ts`, which parses the baked + * value. One schema for both, so they cannot disagree about what a profile is. + */ +export const EnvProfileSchema = z + .object({ + $schema: z.string().optional(), + NIXMAC_ENV: z.enum(NIXMAC_ENVS), + NIXMAC_VERSION: optionalEnvString, + VITE_SERVER_URL: optionalEnvString, + VITE_POSTHOG_KEY: optionalEnvString, + VITE_POSTHOG_HOST: z.string().default("https://us.i.posthog.com"), + VITE_NIXMAC_FILESYSTEM: envBool.default(false), + NIX_INSTALLED_OVERRIDE: envBool.default(false), + NIXMAC_DISABLE_UPDATER: envBool.default(false), + VITE_NIXMAC_SKIP_PERMISSIONS: envBool.default(false), + }) + .passthrough(); + +export type EnvProfile = z.infer; diff --git a/apps/native/src/lib/env.test.ts b/apps/native/src/lib/env.test.ts new file mode 100644 index 000000000..4535907dc --- /dev/null +++ b/apps/native/src/lib/env.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { EnvProfileSchema, type NixmacEnv } from "./env-profile-schema"; +import { settings, toSettings } from "./env"; + +function profile(env: NixmacEnv, overrides: Record = {}) { + return EnvProfileSchema.parse({ NIXMAC_ENV: env, ...overrides }); +} + +describe("toSettings", () => { + it("refuses to skip permissions in a production build, whatever the profile says", () => { + const settingsForProd = toSettings( + profile("production", { VITE_NIXMAC_SKIP_PERMISSIONS: true }), + ); + expect(settingsForProd.skipPermissions).toBe(false); + }); + + it("refuses the nix-installed bypass in a production build", () => { + const settingsForProd = toSettings( + profile("production", { NIX_INSTALLED_OVERRIDE: true }), + ); + expect(settingsForProd.nixInstalledOverride).toBeUndefined(); + }); + + it("honours both bypasses outside production", () => { + for (const env of ["development", "e2e"] as const) { + const bypassed = toSettings( + profile(env, { + VITE_NIXMAC_SKIP_PERMISSIONS: true, + NIX_INSTALLED_OVERRIDE: true, + }), + ); + expect(bypassed.skipPermissions).toBe(true); + expect(bypassed.nixInstalledOverride).toBe(true); + } + }); + + it("leaves the bypasses off when the profile does not ask for them", () => { + for (const env of ["development", "production", "e2e"] as const) { + const plain = toSettings(profile(env)); + expect(plain.skipPermissions).toBe(false); + expect(plain.nixInstalledOverride).toBeUndefined(); + } + }); +}); + +describe("baked profile", () => { + // Fails if the define stops arriving or a fallback profile creeps back in. + // Vitest resolves the defines exactly as the app build does; the selector is + // `development` inside the devenv shell (nix/dev.nix) and unset in CI, and + // both pick env.development.json. + it("resolves to the development profile under vitest", () => { + expect(settings.nixmacEnv).toBe("development"); + }); +}); diff --git a/apps/native/src/lib/env.ts b/apps/native/src/lib/env.ts index bb36e81f6..6fe993f03 100644 --- a/apps/native/src/lib/env.ts +++ b/apps/native/src/lib/env.ts @@ -1,58 +1,11 @@ -import { z } from "zod"; -import development from "../../env.development.json"; -import e2e from "../../env.e2e.json"; -import release from "../../env.release.json"; - -/** Keys from committed `env.{development,release,e2e}.json` (native JSON module types). */ -type EnvProfileKey = - | keyof typeof development - | keyof typeof release - | keyof typeof e2e; - -type ProfileLookupKey = - | EnvProfileKey - | "NIXMAC_VERSION" - | "NIX_INSTALLED_OVERRIDE" - | "VITE_POSTHOG_KEY"; - -declare const __NIXMAC_PROFILE__: "development" | "release" | "e2e"; -declare const __NIXMAC_PROFILE_JSON__: string; - -const envBool = z.preprocess((value) => { - if (typeof value === "boolean") return value; - if (typeof value === "string") { - const normalized = value.trim().toLowerCase(); - return normalized === "true" || normalized === "1" || normalized === "yes"; - } - return false; -}, z.boolean()); - -const optionalEnvString = z.preprocess((value) => { - if (typeof value !== "string") return undefined; - const trimmed = value.trim(); - return trimmed.length > 0 ? trimmed : undefined; -}, z.string().optional()); - -/** Checked-in profile JSON merged with process env at build time (`nixmac-profile.ts`). */ -const EnvProfileSchema = z - .object({ - $schema: z.string().optional(), - NIXMAC_ENV: z.string().default("development"), - NIXMAC_VERSION: optionalEnvString, - VITE_SERVER_URL: optionalEnvString, - VITE_POSTHOG_KEY: optionalEnvString, - VITE_POSTHOG_HOST: z.string().default("https://us.i.posthog.com"), - VITE_NIXMAC_FILESYSTEM: envBool.default(false), - NIX_INSTALLED_OVERRIDE: envBool.default(false), - NIXMAC_DISABLE_UPDATER: envBool.default(false), - VITE_NIXMAC_SKIP_PERMISSIONS: envBool.default(false), - }) - .passthrough(); - -type EnvProfile = z.infer; +import { + EnvProfileSchema, + type EnvProfile, + type NixmacEnv, +} from "./env-profile-schema"; type SettingsType = { - readonly nixmacEnv: string; + readonly nixmacEnv: NixmacEnv; readonly nixmacVersion: string; readonly viteServerUrl?: string; readonly posthogKey?: string; @@ -62,15 +15,25 @@ type SettingsType = { readonly skipPermissions: boolean; }; -function loadMergedProfile(): EnvProfile { - try { - return EnvProfileSchema.parse(JSON.parse(__NIXMAC_PROFILE_JSON__)); - } catch { - return EnvProfileSchema.parse(development); - } +/** + * Whether this profile may switch off gates that exist for real users. + * + * A profile that says `production` never may, whatever else it or the build + * environment asked for. That is the whole of what this function promises: a + * release build gets the right profile in the first place because the selectors + * in `nixmac-profile.ts` and `src-tauri/build.rs` refuse any value they do not + * recognise, not because of anything here. + * + * Rust refuses its own skip-permissions bypass in release builds by a different + * route — `system/permissions.rs` compiles it out. `NIX_INSTALLED_OVERRIDE` + * exists only on this side. + */ +function mayBypassUserGates(env: NixmacEnv): boolean { + return env !== "production"; } -function toSettings(profile: EnvProfile): SettingsType { +export function toSettings(profile: EnvProfile): SettingsType { + const bypassAllowed = mayBypassUserGates(profile.NIXMAC_ENV); return { nixmacEnv: profile.NIXMAC_ENV, nixmacVersion: profile.NIXMAC_VERSION ?? "unknown", @@ -78,15 +41,23 @@ function toSettings(profile: EnvProfile): SettingsType { posthogKey: profile.VITE_POSTHOG_KEY, posthogHost: profile.VITE_POSTHOG_HOST, filesystemEnabled: profile.VITE_NIXMAC_FILESYSTEM, - nixInstalledOverride: profile.NIX_INSTALLED_OVERRIDE ? true : undefined, - skipPermissions: profile.VITE_NIXMAC_SKIP_PERMISSIONS, + nixInstalledOverride: + bypassAllowed && profile.NIX_INSTALLED_OVERRIDE ? true : undefined, + skipPermissions: bypassAllowed && profile.VITE_NIXMAC_SKIP_PERMISSIONS, }; } -const profile = loadMergedProfile(); +/** + * The profile is validated and coerced at build time by `nixmac-profile.ts`; + * parsing it again here checks this module's type rather than asserting it. + * + * There is deliberately no fallback: a profile that fails to parse throws at + * startup rather than silently substituting a different one. + */ +const profile = EnvProfileSchema.parse(__NIXMAC_PROFILE_DATA__); -/** Compile-time profile name selected by `NIXMAC_ENV` (mirrors `build.rs`). */ -export const isE2eProfile = __NIXMAC_PROFILE__ === "e2e"; +/** True only in builds made from `env.e2e.json`. */ +export const isE2eProfile = profile.NIXMAC_ENV === "e2e"; /** Resolved deployment profile for app code. */ export const settings: SettingsType = toSettings(profile); @@ -97,21 +68,6 @@ export const nixmacEnvironment = settings.nixmacEnv; /** App version from the merged profile (`NIXMAC_VERSION`). */ export const nixmacVersion = settings.nixmacVersion; -/** Raw merged profile value for ad-hoc reads. */ -export function getProfileValue( - key: ProfileLookupKey, -): string | boolean | number | undefined { - const value = profile[key as keyof EnvProfile]; - if ( - typeof value === "string" || - typeof value === "boolean" || - typeof value === "number" - ) { - return value; - } - return undefined; -} - export function getWebSiteUrl(): string { return settings.viteServerUrl || "https://nixmac.com"; } diff --git a/apps/native/src/vite-env.d.ts b/apps/native/src/vite-env.d.ts index bb9b37624..6912d7f71 100644 --- a/apps/native/src/vite-env.d.ts +++ b/apps/native/src/vite-env.d.ts @@ -14,5 +14,9 @@ interface ImportMetaEnv { readonly VITEST?: string; } -declare const __NIXMAC_PROFILE__: "development" | "release" | "e2e"; -declare const __NIXMAC_PROFILE_JSON__: string; +/** + * The selected deployment profile, substituted by `nixmac-profile.ts` as a + * JavaScript object literal. Typed `unknown` on purpose: `lib/env.ts` parses it + * against `EnvProfileSchema`, so the shape is checked rather than declared. + */ +declare const __NIXMAC_PROFILE_DATA__: unknown; diff --git a/nix/dev.nix b/nix/dev.nix index e75851642..63650cb02 100644 --- a/nix/dev.nix +++ b/nix/dev.nix @@ -109,11 +109,11 @@ lib.mkIf (!config.container.isBuilding) { export LC_ALL=en_US.UTF-8 export LC_COLLATE=C - # Indicate local development environment (for logging, etc.) - export NIXMAC_ENV=local - export VITE_NIXMAC_ENV=local + # Indicate local development environment (for logging, etc.). + # NIXMAC_ENV selects the committed profile, so it must be one of the names + # apps/native/nixmac-profile.ts and src-tauri/build.rs accept. + export NIXMAC_ENV=development export NIXMAC_VERSION=local-$(whoami) - export VITE_NIXMAC_VERSION=local-$(whoami) # eval "$(starship init $SHELL)" '' diff --git a/scripts/env.sh b/scripts/env.sh index 761afa590..1b465cbae 100644 --- a/scripts/env.sh +++ b/scripts/env.sh @@ -39,13 +39,13 @@ select_env() { No environment selected, which \ environment do you want to use?" if [ -z "$SELECTED_ENV" ]; then - SELECTED_ENV="$(gum choose "prod" "dev" --header="$CHOOSE_MSG")" + SELECTED_ENV="$(gum choose "production" "development" --header="$CHOOSE_MSG")" fi export NIXMAC_ENV="$SELECTED_ENV" } get_secrets_file() { - if [ "$NIXMAC_ENV" == "prod" ]; then + if [ "$NIXMAC_ENV" == "production" ]; then echo "ops/secrets/secrets.yaml" else echo "ops/secrets/secrets.dev.yaml"