Skip to content

Commit 0b2c779

Browse files
committed
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
1 parent 144568d commit 0b2c779

20 files changed

Lines changed: 352 additions & 141 deletions

File tree

.cursor/rules/native-env.mdc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ alwaysApply: false
88

99
**Do not use `process.env` or `import.meta.env` anywhere except `apps/native/src/lib/env.ts`.**
1010

11-
All app code reads deployment settings through exports from that module (`settings`, `nixmacEnvironment`, `getProfileValue`, etc.).
11+
All app code reads deployment settings through exports from that module (`settings`, `nixmacEnvironment`, `nixmacVersion`, `isE2eProfile`).
1212

1313
Benefits: single validation path, typed profile JSON, no scattered env reads, and build-time profile baking stays consistent with Rust (`build.rs`).
1414

.github/workflows/build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ jobs:
8787
working-directory: apps/native
8888
run: cargo test --manifest-path src-tauri/Cargo.toml
8989
env:
90-
NIXMAC_ENV: production
90+
NIXMAC_ENV: prod
9191
build:
9292
needs: rust-tests
9393
runs-on: [self-hosted, macOS]

apps/native/env.release.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"$schema": "src-tauri/resources/schemas/env.schema.json",
3-
"NIXMAC_ENV": "production",
3+
"NIXMAC_ENV": "prod",
44
"VITE_SERVER_URL": "https://nixmac.com",
55
"VITE_POSTHOG_HOST": "https://us.i.posthog.com",
66
"VITE_POSTHOG_KEY": "phc_qQ8JdjX8fo6Viodke6c4hm2Wb3ohKW3JuMUX3CjeyUcp",

apps/native/nixmac-profile.ts

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { readFileSync } from "node:fs";
22
import path from "node:path";
3+
import { EnvProfileSchema, NIXMAC_ENVS } from "./src/lib/env-profile-schema";
34

45
type NixmacProfileName = "development" | "release" | "e2e";
56

@@ -8,16 +9,26 @@ function readProfileJson(nativeAppDir: string, name: NixmacProfileName): Record<
89
return JSON.parse(raw) as Record<string, unknown>;
910
}
1011

11-
/** Profile file selection — keep in sync with `apps/native/src-tauri/build.rs`. */
12+
/**
13+
* Profile file selection — keep in sync with `apps/native/src-tauri/build.rs`.
14+
*
15+
* Unset means development. Any other value is a mistake in the build command,
16+
* not a request for the default: falling through used to bake the development
17+
* profile, permission checks disabled, into a build that looked like a release.
18+
*/
1219
function resolveNixmacProfile(): NixmacProfileName {
13-
switch (process.env.NIXMAC_ENV ?? "development") {
20+
const selector = process.env.NIXMAC_ENV ?? "development";
21+
switch (selector) {
22+
case "development":
23+
return "development";
1424
case "prod":
15-
case "production":
1625
return "release";
1726
case "e2e":
1827
return "e2e";
1928
default:
20-
return "development";
29+
throw new Error(
30+
`NIXMAC_ENV must be unset or one of ${NIXMAC_ENVS.join(", ")}; got ${JSON.stringify(selector)}`,
31+
);
2132
}
2233
}
2334

@@ -65,6 +76,15 @@ const OVERRIDABLE_PREFIXES = [
6576
"NIX_INSTALLED_",
6677
] as const;
6778

79+
/**
80+
* Keys process env must never overwrite.
81+
*
82+
* `NIXMAC_ENV` picks which profile file to read; letting it also overwrite that
83+
* file's own `NIXMAC_ENV` value made the two disagree — `NIXMAC_ENV=prod` stored
84+
* "prod" where `env.release.json` said "production". A selector is not a setting.
85+
*/
86+
const NON_OVERRIDABLE_KEYS = new Set(["$schema", "NIXMAC_ENV"]);
87+
6888
function isOverridableKey(key: string): boolean {
6989
return OVERRIDABLE_PREFIXES.some((prefix) => key.startsWith(prefix));
7090
}
@@ -80,7 +100,8 @@ function mergeProfileWithProcessEnv(
80100
const merged: Record<string, unknown> = { ...base };
81101

82102
for (const [key, envValue] of Object.entries(process.env)) {
83-
if (key === "$schema" || envValue === undefined || envValue.trim() === "") continue;
103+
if (NON_OVERRIDABLE_KEYS.has(key) || envValue === undefined || envValue.trim() === "")
104+
continue;
84105
if (!(key in merged) && !isOverridableKey(key)) continue;
85106
merged[key] = coerceEnvOverride(merged[key], envValue);
86107
}
@@ -101,11 +122,17 @@ function resolveMergedProfile(nativeAppDir: string): Record<string, unknown> {
101122
return mergeProfileWithProcessEnv(base, nativeAppDir);
102123
}
103124

125+
/**
126+
* Vite `define` entries that bake the selected profile into the bundle.
127+
*
128+
* A define is raw text substitution, so this emits a JavaScript object literal
129+
* that `src/lib/env.ts` consumes directly: no string to parse, and therefore no
130+
* parse for a bad profile to fall back from. Validating and coercing here means
131+
* an invalid profile fails the build instead of the app.
132+
*/
104133
export function nixmacBuildDefines(nativeAppDir: string): Record<string, string> {
105-
const profileName = resolveNixmacProfile();
106-
const merged = resolveMergedProfile(nativeAppDir);
134+
const profile = EnvProfileSchema.parse(resolveMergedProfile(nativeAppDir));
107135
return {
108-
__NIXMAC_PROFILE__: JSON.stringify(profileName),
109-
__NIXMAC_PROFILE_JSON__: JSON.stringify(merged),
136+
__NIXMAC_PROFILE_DATA__: JSON.stringify(profile),
110137
};
111138
}

apps/native/src-tauri/build.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,21 @@ use std::path::Path;
77
use std::process::Command;
88

99
/// Embed `apps/native/env.{development,release,e2e}.json` selected by `NIXMAC_ENV`.
10+
///
11+
/// Accepted values must stay in sync with `apps/native/nixmac-profile.ts`.
12+
/// Unset means development; anything else stops the build. Falling through to
13+
/// the development profile is how a mistyped selector used to produce a build
14+
/// that looked like a release but carried the development profile.
1015
fn embed_build_profile() {
1116
let native_app_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("..");
12-
let profile = std::env::var("NIXMAC_ENV").unwrap_or_else(|_| "development".to_string());
13-
let file = match profile.as_str() {
14-
"prod" | "production" => "env.release.json",
17+
let selector = std::env::var("NIXMAC_ENV").unwrap_or_else(|_| "development".to_string());
18+
let file = match selector.as_str() {
19+
"development" => "env.development.json",
20+
"prod" => "env.release.json",
1521
"e2e" => "env.e2e.json",
16-
_ => "env.development.json",
22+
other => panic!(
23+
"NIXMAC_ENV must be unset or one of development, prod, e2e; got {other:?}"
24+
),
1725
};
1826
let path = native_app_dir.join(file);
1927

@@ -25,11 +33,14 @@ fn embed_build_profile() {
2533
);
2634
}
2735

28-
let json = std::fs::read_to_string(&path).unwrap_or_else(|_| "{}".to_string());
29-
let minified = serde_json::from_str::<serde_json::Value>(&json)
30-
.ok()
31-
.and_then(|value| serde_json::to_string(&value).ok())
32-
.unwrap_or_else(|| "{}".to_string());
36+
// An unreadable or malformed profile stops the build. Degrading to "{}"
37+
// compiles an app whose every setting silently falls back to its default.
38+
let json = std::fs::read_to_string(&path)
39+
.unwrap_or_else(|error| panic!("cannot read {}: {error}", path.display()));
40+
let value: serde_json::Value = serde_json::from_str(&json)
41+
.unwrap_or_else(|error| panic!("cannot parse {}: {error}", path.display()));
42+
let minified = serde_json::to_string(&value)
43+
.unwrap_or_else(|error| panic!("cannot re-encode {}: {error}", path.display()));
3344
println!("cargo:rustc-env=NIXMAC_ENV_PROFILE_JSON={minified}");
3445
}
3546

apps/native/src-tauri/src/env/config.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,12 @@ pub struct NixmacEnvSettings {
5252
)]
5353
pub sentry_dsn: String,
5454

55+
/// No `build_embed`: `NIXMAC_ENV` selects which profile to embed, so
56+
/// embedding it as a value too let the selector overwrite the value it
57+
/// selected. It resolves from the profile JSON instead. A process-env
58+
/// override still wins here, as it does for every field in this struct.
5559
#[config(
5660
default = "prod",
57-
build_embed = true,
5861
env_var = "NIXMAC_ENV",
5962
label = "Deployment environment"
6063
)]

apps/native/src-tauri/src/env/sources.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ pub fn build_embed(name: &str) -> Option<String> {
1616
"SENTRY_DSN" => option_env!("SENTRY_DSN").map(str::to_string),
1717
"VITE_SERVER_URL" => option_env!("VITE_SERVER_URL").map(str::to_string),
1818
"SUBMITTED_FEEDBACK_DSN" => option_env!("SUBMITTED_FEEDBACK_DSN").map(str::to_string),
19-
"NIXMAC_ENV" => option_env!("NIXMAC_ENV").map(str::to_string),
2019
"NIXMAC_VERSION" => option_env!("NIXMAC_VERSION").map(str::to_string),
2120
_ => None,
2221
};
@@ -25,8 +24,16 @@ pub fn build_embed(name: &str) -> Option<String> {
2524
}
2625

2726
/// JSON profile from `apps/native/env.{development,release,e2e}.json`, embedded at compile time.
27+
///
28+
/// `None` only when nothing was embedded. A malformed embed panics rather than
29+
/// resolving to `None`, which would silently drop every profile value back to
30+
/// its field default.
2831
pub fn build_profile() -> Option<serde_json::Value> {
29-
option_env!("NIXMAC_ENV_PROFILE_JSON").and_then(|raw| serde_json::from_str(raw).ok())
32+
let raw = option_env!("NIXMAC_ENV_PROFILE_JSON")?;
33+
Some(
34+
serde_json::from_str(raw)
35+
.expect("NIXMAC_ENV_PROFILE_JSON is minified by build.rs and must parse"),
36+
)
3037
}
3138

3239
#[cfg(test)]

apps/native/src-tauri/src/env_keys.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44

55
#[allow(dead_code)]
66
/// Environment variables embedded at build time via `build.rs` (`cargo:rustc-env`).
7-
pub const BUILD_EMBED_KEYS: &[&str] = &[
8-
"SENTRY_DSN",
9-
"VITE_SERVER_URL",
10-
"SUBMITTED_FEEDBACK_DSN",
11-
"NIXMAC_ENV",
12-
];
7+
/// `NIXMAC_ENV` is deliberately absent: it selects which profile to embed, so
8+
/// embedding it as a value too let the selector overwrite the selected file's
9+
/// own `NIXMAC_ENV`. The value comes from the embedded profile JSON instead.
10+
pub const BUILD_EMBED_KEYS: &[&str] = &["SENTRY_DSN", "VITE_SERVER_URL", "SUBMITTED_FEEDBACK_DSN"];
1311

1412
/// Application environment variable names.
1513
#[allow(dead_code)]

apps/native/src-tauri/src/main.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -502,12 +502,9 @@ fn run_gui_mode(
502502
std::sync::Mutex<Option<tracing_appender::non_blocking::WorkerGuard>>,
503503
>,
504504
) {
505-
// Prefer compile-time embedded vars (set by build.rs via `cargo:rustc-env`),
506-
// fall back to runtime environment variables.
507-
let nixmac_env = option_env!("NIXMAC_ENV")
508-
.map(|s| s.to_string())
509-
.or_else(|| std::env::var("NIXMAC_ENV").ok())
510-
.unwrap_or_else(|| "prod".to_string());
505+
// One resolution path for the deployment profile; `crate::env` owns the
506+
// precedence so this log line cannot disagree with what telemetry reports.
507+
let nixmac_env = crate::env::nixmac_env();
511508

512509
let nixmac_version = option_env!("NIXMAC_VERSION")
513510
.map(|s| s.to_string())

apps/native/src-tauri/src/system/permissions.rs

Lines changed: 76 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -238,28 +238,46 @@ fn check_folder_access(path: &PathBuf) -> PermissionStatus {
238238
}
239239
}
240240

241-
/// Check if we have Full Disk Access.
241+
/// Shown on the Full Disk Access row when every probe path was missing.
242+
///
243+
/// The probe reads files a granted process can read and a denied one cannot.
244+
/// If none of them exist there is nothing to read, so it proves neither state —
245+
/// say so, rather than implying we checked and found the access missing. The
246+
/// other inconclusive path, a Mac with no home directory, has its own message.
247+
const FULL_DISK_ACCESS_INCONCLUSIVE: &str = "Could not determine Full Disk Access: none of the files this check probes exist on this Mac. If a rebuild fails with a permissions error, add nixmac under System Settings → Privacy & Security → Full Disk Access.";
248+
249+
/// Check if we have Full Disk Access, with an explanation when inconclusive.
242250
///
243251
/// Probes several TCC-gated paths. A successful read on any one is proof of
244252
/// FDA. A PermissionDenied on any one is proof of the opposite — even if the
245253
/// user has nixmac listed and toggled on in System Settings, a stale TCC
246254
/// entry (e.g. codesign requirement mismatch after an update-in-place) can
247255
/// leave the grant silently inactive, and reads will fail with
248-
/// PermissionDenied. Only if every probe path is missing (NotFound) do we
249-
/// fall back to Pending.
250-
fn check_full_disk_access() -> PermissionStatus {
256+
/// PermissionDenied. If every probe path is missing (NotFound) the probe has
257+
/// established nothing, which is `Unknown` rather than `Pending` — the latter
258+
/// reads as "not granted yet", which is a claim this probe cannot make.
259+
///
260+
/// `Unknown` still fails the onboarding gate on its own; what keeps an
261+
/// unverifiable result from holding the gate shut is the caller dropping the
262+
/// row's `required` flag (see the `full-disk` arm of `check_all_permissions`).
263+
fn check_full_disk_access() -> (PermissionStatus, Option<String>) {
251264
if vite_skip_permissions_enabled() {
252265
debug!("VITE_NIXMAC_SKIP_PERMISSIONS is set, assuming Full Disk Access granted");
253-
return PermissionStatus::Granted;
266+
return (PermissionStatus::Granted, None);
254267
}
255268
if e2e_skip_permissions_enabled() {
256269
debug!("E2E permission skip enabled, assuming Full Disk Access granted");
257-
return PermissionStatus::Granted;
270+
return (PermissionStatus::Granted, None);
258271
}
259272

260273
let home = match dirs::home_dir() {
261274
Some(h) => h,
262-
None => return PermissionStatus::Unknown,
275+
None => {
276+
return (
277+
PermissionStatus::Unknown,
278+
Some("Could not determine Full Disk Access: this Mac reported no home directory, so the check could not run. If a rebuild fails with a permissions error, add nixmac under System Settings → Privacy & Security → Full Disk Access.".to_string()),
279+
);
280+
}
263281
};
264282

265283
// (path, is_dir). Ordered by how reliably the path exists on a typical
@@ -286,7 +304,7 @@ fn check_full_disk_access() -> PermissionStatus {
286304
match result {
287305
Ok(_) => {
288306
debug!("Full Disk Access granted (probe succeeded: {:?})", path);
289-
return PermissionStatus::Granted;
307+
return (PermissionStatus::Granted, None);
290308
}
291309
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
292310
debug!("Full Disk Access denied (probe blocked: {:?})", path);
@@ -297,10 +315,13 @@ fn check_full_disk_access() -> PermissionStatus {
297315
}
298316

299317
if saw_denied {
300-
PermissionStatus::Denied
318+
(PermissionStatus::Denied, None)
301319
} else {
302320
debug!("Full Disk Access check inconclusive — no probe path existed");
303-
PermissionStatus::Pending
321+
(
322+
PermissionStatus::Unknown,
323+
Some(FULL_DISK_ACCESS_INCONCLUSIVE.to_string()),
324+
)
304325
}
305326
}
306327

@@ -345,7 +366,24 @@ pub fn check_all_permissions() -> PermissionsState {
345366
"desktop" => check_desktop_access(),
346367
"documents" => check_documents_access(),
347368
"admin" => check_admin_privileges(),
348-
"full-disk" => check_full_disk_access(),
369+
"full-disk" => {
370+
let (status, detail) = check_full_disk_access();
371+
// Keep the default "how to grant it" text unless the probe has
372+
// something more accurate to say.
373+
if let Some(detail) = detail {
374+
perm.instructions = Some(detail);
375+
}
376+
// A probe that could not decide must not keep the gate shut —
377+
// the same reasoning that makes app-management Recommended
378+
// rather than Required (see `app_management_permission`), except
379+
// discovered at runtime. Requiring access we cannot verify shows
380+
// a permissions banner to users who may well have granted it.
381+
// Only this row relaxes: every other `Unknown` still blocks.
382+
if status == PermissionStatus::Unknown {
383+
perm.required = false;
384+
}
385+
status
386+
}
349387
"app-management" => check_app_management(),
350388
"privileged-helper" => {
351389
let (status, detail) = check_privileged_helper();
@@ -356,7 +394,10 @@ pub fn check_all_permissions() -> PermissionsState {
356394
}
357395
status
358396
}
359-
_ => PermissionStatus::Unknown,
397+
other => {
398+
debug_assert!(false, "no probe for permission id {other:?}");
399+
PermissionStatus::Unknown
400+
}
360401
};
361402

362403
// If this permission is required and not granted, mark all_required_granted as false
@@ -501,17 +542,18 @@ pub fn request_permission(permission_id: &str) -> Result<Permission> {
501542
.spawn();
502543

503544
// Re-check the status
545+
let (status, detail) = check_full_disk_access();
504546
Ok(Permission {
505547
id: "full-disk".to_string(),
506548
name: "Full Disk Access".to_string(),
507549
description: "Required for darwin-rebuild to apply system changes".to_string(),
508550
required: true,
509551
can_request_programmatically: false,
510-
status: check_full_disk_access(),
511-
instructions: Some(
552+
status,
553+
instructions: Some(detail.unwrap_or_else(|| {
512554
"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."
513-
.to_string(),
514-
),
555+
.to_string()
556+
})),
515557
})
516558
}
517559
"app-management" => {
@@ -805,6 +847,24 @@ mod tests {
805847
);
806848
}
807849

850+
#[test]
851+
fn inconclusive_full_disk_access_is_unknown_and_explains_itself() {
852+
// Only meaningful when the probe is actually inconclusive; on a Mac
853+
// with Safari or Mail the probe reaches a verdict and this is skipped.
854+
// Either inconclusive path must explain itself — which one ran depends
855+
// on the machine, so assert the property rather than the wording.
856+
let (status, detail) = check_full_disk_access();
857+
if status == PermissionStatus::Unknown {
858+
let detail = detail.expect("an inconclusive probe must explain itself");
859+
assert!(detail.starts_with("Could not determine Full Disk Access:"));
860+
} else {
861+
assert!(matches!(
862+
status,
863+
PermissionStatus::Granted | PermissionStatus::Denied
864+
));
865+
}
866+
}
867+
808868
#[test]
809869
fn app_management_is_not_inferred_from_full_disk_access() {
810870
assert_eq!(check_app_management(), PermissionStatus::Pending);

0 commit comments

Comments
 (0)