SecretRead migration + threat-model refresh for core-dump, handle wrap, bridge

.github/workflows/ci.yml

@@ -74,4 +74,12 @@ jobs:
         run: echo "TMPDIR=$(python3 -c 'import tempfile, os; print(os.path.realpath(tempfile.gettempdir()))')" >> "$GITHUB_ENV"
 
       - name: Test
+        env:
+          # Force the in-memory MockEncryptionStorage on macOS runners.
+          # A real SecItemAdd to the runner's login keychain from an
+          # unsigned binary blocks indefinitely on an ACL confirmation
+          # prompt that the headless runner cannot answer. The library
+          # itself still exercises the real keychain path in its own
+          # enclaveapp-apple suite, which does run on macOS CI.
+          ENCLAVEAPP_MOCK_STORAGE: ${{ runner.os == 'macOS' && '1' || '' }}
         run: cargo test --workspace -- --test-threads=1

THREAT_MODEL.md

@@ -94,12 +94,23 @@ apply to the npmenc process's copy, not npm's copy.
 
 ### Threat: Core dump / swap of the npm process
 
-npmenc calls `harden_process()` to disable core dumps of itself, but the
-target `npm` process is a separate PID. npmenc does not set rlimits on
-the target. A system-wide coredump policy can still capture `npm`'s env.
-
-**Mitigations:** operator-level `sysctl kernel.core_pattern=` or
-`ulimit -c 0` in the user's shell environment.
+npmenc calls `harden_process()` to disable core dumps of itself, and now
+also clamps `RLIMIT_CORE` on the spawned `npm`/`npx` child via a
+`pre_exec` hook in the launcher (`crates/enclaveapp-app-adapter/src/launcher.rs`).
+A segfault of `npm` no longer produces a core dump containing the
+interpolated `NPM_TOKEN_*` env bytes.
+
+**Mitigations:**
+- `disable_core_dumps_in_child` inside the launcher calls
+  `setrlimit(RLIMIT_CORE, {0, 0})` on Unix immediately before
+  `execve` — the child inherits the clamped limit for the rest of
+  its lifetime.
+- Operator-level `sysctl kernel.core_pattern=` or `ulimit -c 0`
+  remains a defense-in-depth backup.
+
+**Residual risk:** Windows WER crash-dump collection is a system-wide
+policy; child-process-local opt-out is not available. Users on
+Windows who care should disable WER at the group-policy level.
 
 ### Threat: npm's own logging / telemetry leaks the token
 
@@ -229,14 +240,23 @@ mode. Alias / function resolution has an 8-deep recursion cap.
 resolves. PATH-hijack is a user-side compromise that defeats many
 defenses at once.
 
-### Threat: `.handle` plaintext on macOS
-
-Inherited from libenclaveapp: the macOS Secure Enclave `.handle` file
-is currently plaintext on disk (0600). Keychain-wrapped AES-GCM is a
-planned hardening (`libenclaveapp/fix-macos.md`). Same-UID handle theft
-is possible on macOS until that lands — an attacker who copies the
-handle can replay SE signing on that user's device, defeating npmenc's
-"token on another host is useless" guarantee at the local level.
+### Threat: `.handle` same-UID theft on macOS
+
+Inherited from libenclaveapp: the macOS Secure Enclave `.handle`
+file is now AES-256-GCM sealed under a per-label wrapping key held
+in the login Keychain (`libenclaveapp/crates/enclaveapp-apple/src/keychain_wrap.rs`,
+on-disk format `[4B "EHW1"][12B nonce][ciphertext][16B tag]`). A
+same-UID attacker who copies the raw `.handle` file cannot replay SE
+operations without also having the Keychain wrapping key — and the
+Keychain's code-signature-bound ACL blocks that access from any
+binary other than the one that created it, reprompting the user on
+any unfamiliar binary. On ad-hoc (unsigned) builds every rebuild
+prompts once; on trusted-signing-identity builds the prompt is
+one-time per install. Residual risk reduces to the same-UID attacker
+who can *both* read the `.handle` file *and* drive the Keychain from
+the legitimate `npmenc` binary — equivalent to running `npmenc`
+directly as the user, which is outside what any Type-2 design can
+defend against.
 
 ### Threat: Multi-user machines
 
@@ -254,9 +274,13 @@ Windows credentials.
 ### Threat: WSL bridge
 
 Same as libenclaveapp — the bridge binary is discovered by a fixed-path
-list under `/mnt/c/Program Files/npmenc/`, with a `which` fallback that
-accepts any user-writable directory earlier on `$PATH`. PE signature
-validation is a tracked hardening gap. See the
+list under `/mnt/c/Program Files/npmenc/`. The former `which`-based
+PATH fallback has been removed so a user-writable `$PATH` entry cannot
+substitute a malicious bridge. Before spawn, the bridge binary's PE
+header is parsed and rejected if it carries no Authenticode signature
+block (opt-out `ENCLAVEAPP_BRIDGE_ALLOW_UNSIGNED=1`). Full chain
+verification via `WinVerifyTrust` is not reachable from the WSL side
+and is the acknowledged residual risk. See the
 [libenclaveapp threat model](https://github.com/godaddy/libenclaveapp/blob/main/THREAT_MODEL.md)
 for details.
 
@@ -266,13 +290,18 @@ Ranked by realistic impact on an npm user today:
 
 1. **Malicious dependency lifecycle script exfiltrates `NPM_TOKEN_*`.**
    `npm install` runs untrusted JavaScript with the token in the
-   environment. No defense possible at the npmenc layer.
+   environment. No defense possible at the npmenc layer. Users can
+   reduce exposure with `npmenc --publish-only` on install/ci paths.
 2. **Same-UID process reads `/proc/<npm-PID>/environ`.** Any concurrently
    running same-user process sees the token for npm's full lifetime.
-3. **npm itself leaks the token** through telemetry, crash dumps, or
+3. **Inherited `NPM_TOKEN_*` in the parent shell.** A developer who
+   has `NPM_TOKEN=...` exported in their shell gets that value
+   propagated to the wrapped `npm` child regardless of npmenc's own
+   encrypted binding. Callers can opt into scrubbing via the
+   launcher's `with_env_scrub(["NPM_TOKEN_*"])` (`enclaveapp-app-adapter::launcher`)
+   to remove inherited matches before spawn.
+4. **npm itself leaks the token** through telemetry, crash dumps, or
    verbose logging modes the user opts into.
-4. **`.handle` plaintext on macOS** until `fix-macos.md` lands —
-   same-UID handle theft allows local SE replay.
 5. **Token replay** within the npm automation token's validity window —
    rotate suspect tokens, prefer short-lived granular tokens.
 

npmenc-core/src/passthrough.rs

@@ -99,6 +99,7 @@ pub fn prepare_passthrough(
             args,
             env_overrides,
             env_removals: Vec::new(),
+            env_scrub_patterns: Vec::new(),
         },
         effective_config_path: effective_userconfig,
         effective_config_contents: source,

npmenc-core/src/token_source.rs

@@ -6,8 +6,8 @@ use std::process::Command;
 
 use anyhow::{anyhow, Result};
 use enclaveapp_app_adapter::{
-    resolve_program, BindingId, BindingRecord, ResolveMode, ResolveOptions, SecretStore,
-    REDACTED_PLACEHOLDER,
+    resolve_program, BindingId, BindingRecord, ResolveMode, ResolveOptions, SecretRead,
+    SecretStore, REDACTED_PLACEHOLDER,
 };
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
@@ -580,28 +580,36 @@ where
     let Some(expected_provider) = record.metadata.get(TOKEN_PROVIDER_KEY) else {
         return Ok(None);
     };
-    let Some(raw_state) = secret_store.get(&token_source_state_id(&record.id))? else {
-        return Err(anyhow!(
-            "binding `{}` has token source metadata but its persisted token source state is missing",
-            record.label
-        ));
+    let raw_state = match secret_store.get_read(&token_source_state_id(&record.id))? {
+        SecretRead::Present(value) => value,
+        SecretRead::Redacted => {
+            // Inspection path: the underlying secret exists but the
+            // store won't decrypt. Surface the appropriate placeholder
+            // state for the two supported providers. Note we never see
+            // Redacted on the read-write store, so this is reached
+            // only through the read-only inspection store.
+            return Ok(match (expected_provider.as_str(), read_mode) {
+                (COMMAND_TOKEN_PROVIDER, _) => Some(BindingTokenSource::Command {
+                    command: REDACTED_PLACEHOLDER.to_string(),
+                    prepared: true,
+                    needs_persist: false,
+                }),
+                (_, TokenSourceReadMode::Inspection) => Some(BindingTokenSource::Provider {
+                    provider: expected_provider.clone(),
+                    handle: None,
+                    adapter: None,
+                    needs_persist: false,
+                }),
+                _ => None,
+            });
+        }
+        SecretRead::Absent => {
+            return Err(anyhow!(
+                "binding `{}` has token source metadata but its persisted token source state is missing",
+                record.label
+            ));
+        }
     };
-    if raw_state == REDACTED_PLACEHOLDER {
-        return Ok(match (expected_provider.as_str(), read_mode) {
-            (COMMAND_TOKEN_PROVIDER, _) => Some(BindingTokenSource::Command {
-                command: raw_state,
-                prepared: true,
-                needs_persist: false,
-            }),
-            (_, TokenSourceReadMode::Inspection) => Some(BindingTokenSource::Provider {
-                provider: expected_provider.clone(),
-                handle: None,
-                adapter: None,
-                needs_persist: false,
-            }),
-            _ => None,
-        });
-    }
     if !raw_state.trim_start().starts_with('{') {
         return Err(anyhow!(
             "binding `{}` has token source metadata but its persisted token source state is in an unsupported legacy format",
@@ -638,11 +646,22 @@ fn load_token_source_state_by_id<S>(
 where
     S: SecretStore,
 {
-    let Some(state) = secret_store.get(&token_source_state_id(binding_id))? else {
-        return Ok(None);
+    let state = match secret_store.get_read(&token_source_state_id(binding_id))? {
+        SecretRead::Present(value) => value,
+        SecretRead::Redacted => {
+            // Read-only store indicating a real entry exists but is
+            // intentionally not handed over. Return the canonical
+            // redacted-command state without round-tripping through
+            // the string sentinel.
+            return Ok(Some(BindingTokenSource::Command {
+                command: REDACTED_PLACEHOLDER.to_string(),
+                prepared: true,
+                needs_persist: false,
+            }));
+        }
+        SecretRead::Absent => return Ok(None),
     };
-    if state != REDACTED_PLACEHOLDER
-        && !state.trim_start().starts_with('{')
+    if !state.trim_start().starts_with('{')
         && !has_prepared_token_source_state(binding_id, secret_store)?
     {
         return Err(anyhow!(
@@ -653,13 +672,6 @@ where
 }
 
 fn load_token_source_state_by_id_from_value(state: String) -> Result<Option<BindingTokenSource>> {
-    if state == REDACTED_PLACEHOLDER {
-        return Ok(Some(BindingTokenSource::Command {
-            command: state,
-            prepared: true,
-            needs_persist: false,
-        }));
-    }
     if !state.trim_start().starts_with('{') {
         return Ok(Some(parse_legacy_prepared_command_state(&state)?));
     }
@@ -1902,9 +1914,16 @@ mod tests {
     fn inspection_reacquires_redacted_provider_state_when_prepared_marker_exists() {
         let secrets = MemorySecretStore::new();
         let record = binding_record_with_provider("sso-jwt");
+        // Simulate the ReadOnly-inspection-store outcome: a state
+        // entry exists but the store won't hand over the plaintext.
+        // `mark_redacted` is the test-only equivalent of what
+        // ReadOnlyEncryptedFileSecretStore::get_read returns — it
+        // does NOT round-trip through the `"<redacted>"` string
+        // sentinel, which is the collision the typed SecretRead API
+        // was added to eliminate.
         secrets
-            .set(&token_source_state_id(&record.id), REDACTED_PLACEHOLDER)
-            .expect("state");
+            .mark_redacted(&token_source_state_id(&record.id))
+            .expect("mark redacted");
         secrets
             .set(
                 &token_source_prepared_id(&record.id),