From 51b61dbc0b1aa4e1071811d8404da49631d285fc Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Thu, 16 Apr 2026 22:13:52 -0700 Subject: [PATCH 1/2] Add threat model and make .npmrc rewrites atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This repo previously shipped only DESIGN.md — no dedicated threat model and no security policy. Add THREAT_MODEL.md that names what npmenc does and doesn't protect against under Type 2 (EnvInterpolation) delivery, with explicit focus on: - The defining residual risk: every npm install runs arbitrary JS from transitive dependency lifecycle scripts with NPM_TOKEN_* in the environment. This is the single biggest threat specific to npm as a Type 2 target and cannot be mitigated inside npmenc. - Same-UID /proc//environ readability for npm's full lifetime. - npmenc-specific concerns: rollback via VCS-stored plaintext .npmrc, atomic rewrites, concurrent npm vs. npmenc, HTTPS-only registry URL reconstruction, over-injection of NPM_TOKEN_* on subcommands that don't need it, token-source timeout, sentinel, program resolution, macOS .handle plaintext, multi-user systems, WSL bridge. Add SECURITY.md with the usual reporting-vulnerabilities boilerplate plus a summary of guarantees and limitations, mirroring the sister projects' style. Make .npmrc rewrites atomic. The five fs::write call sites in install.rs and uninstall.rs are replaced with a new npmenc-core::atomic_write::atomic_write_preserving_mode that writes to a sibling temp file and renames over the target, preserving the original file's Unix mode bits. Power loss / crash mid-rewrite now leaves either the pre-install or post-install contents, never a partial mix. 4 unit tests added for the atomic-write helper; existing install / uninstall tests continue to pass. No public API changes. --- Cargo.lock | 1 + SECURITY.md | 91 +++++++++++ THREAT_MODEL.md | 272 ++++++++++++++++++++++++++++++++ npmenc-core/src/atomic_write.rs | 108 +++++++++++++ npmenc-core/src/install.rs | 5 +- npmenc-core/src/lib.rs | 1 + npmenc-core/src/uninstall.rs | 9 +- 7 files changed, 482 insertions(+), 5 deletions(-) create mode 100644 SECURITY.md create mode 100644 THREAT_MODEL.md create mode 100644 npmenc-core/src/atomic_write.rs diff --git a/Cargo.lock b/Cargo.lock index 73733e1..eee81c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -412,6 +412,7 @@ dependencies = [ "tempfile", "thiserror", "tracing", + "zeroize", ] [[package]] diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..7adac55 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,91 @@ +# Security Policy + +## Reporting Vulnerabilities + +If you discover a security vulnerability in npmenc, report it privately. + +**Do not open a public GitHub issue for security vulnerabilities.** + +Email: Report via GitHub's private vulnerability reporting feature on the +[npmenc repository](https://github.com/godaddy/npmenc/security/advisories/new), +or contact the maintainer directly. + +Include: +- Description of the vulnerability +- Steps to reproduce +- Potential impact +- Suggested fix (if you have one) + +You will receive an acknowledgment within 72 hours. A fix will be developed +and released as quickly as possible, with credit given to the reporter +(unless anonymity is requested). + +## Supported Versions + +| Version | Supported | +|---|---| +| 0.1.x | Yes | + +Only the latest release receives security fixes. + +## Security Model Summary + +`npmenc` / `npxenc` are [Type 2 (EnvInterpolation)](https://github.com/godaddy/libenclaveapp/blob/main/DESIGN.md#type-2-envinterpolation) +wrappers around `npm` and `npx`. Registry auth tokens are encrypted at +rest under a hardware-bound P-256 key; at run time, `.npmrc` placeholders +are resolved to `${NPM_TOKEN_*}` env vars that are passed to the real +npm binary via `execve()`. + +- **Hardware-backed encryption at rest.** Tokens are ECIES-encrypted + under a Secure Enclave (macOS), TPM 2.0 (Windows / Linux), or + keyring-wrapped software key. The private key never leaves the + hardware on SE / TPM backends. +- **No plaintext tokens on disk after install.** `.npmrc` contains only + `${NPM_TOKEN_*}` placeholders; the encrypted token lives in + `$NPMENC_CONFIG_DIR/secrets/`. +- **Atomic rewrites.** `.npmrc` rewrites go through tmp-then-rename with + preserved mode bits (`npmenc-core::atomic_write`). A crash or power + loss cannot leave a partially-written config. +- **Direct exec, no shell.** npm / npx are launched via + `Command::new(path).env(...)`, never via `sh -c`. +- **In-memory secret hygiene.** The launcher `mlock`s env-var bytes and + zeroizes them after the child exits (`enclaveapp-app-adapter`). +- **Core dumps disabled.** `harden_process()` sets `RLIMIT_CORE = 0` for + the npmenc process. +- **Type-2-limit fundamental risks are documented**, not mitigated. See + `THREAT_MODEL.md`. + +### What npmenc does NOT protect against + +- **Malicious npm lifecycle scripts.** `npm install` runs arbitrary JS + from every transitive dep with the token in its environment. This is + the single biggest risk when using npm with any authentication + mechanism, and is outside npmenc's control. Users should prefer + granular / short-lived publish tokens and `--ignore-scripts` on + untrusted trees. +- Same-UID processes reading `/proc//environ`. +- npm's own telemetry / crash reports that include env vars. +- Root / kernel compromise. +- Physical attacks on the Secure Enclave or TPM hardware. +- On macOS, same-UID theft of the Secure Enclave `.handle` file while + Keychain-backed wrapping is still a planned hardening (see + [libenclaveapp/fix-macos.md](https://github.com/godaddy/libenclaveapp/blob/main/fix-macos.md)). + +See [THREAT_MODEL.md](THREAT_MODEL.md) for detailed analysis and +[libenclaveapp/THREAT_MODEL.md](https://github.com/godaddy/libenclaveapp/blob/main/THREAT_MODEL.md) +for the shared foundation. + +## Dependencies + +npmenc uses a conservative set of dependencies. Key external crates: + +- `enclaveapp-*`: Shared hardware-backed key management (libenclaveapp) +- `anyhow`, `clap`: Error handling and CLI +- `serde`, `serde_json`, `toml`: Serialization +- `sha2`: Hashing for secret-file naming +- `shlex`: Safe shell-word tokenization for token-source helpers +- `fs4`: File locking for state coordination +- `tempfile`: Atomic file writes (named-temp-file + persist) + +All dependencies are published on crates.io and are widely used in the +Rust ecosystem. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md new file mode 100644 index 0000000..eb85413 --- /dev/null +++ b/THREAT_MODEL.md @@ -0,0 +1,272 @@ +# Threat Model: npmenc / npxenc + +## Scope + +`npmenc` (and its sibling `npxenc`) is a +[Type 2 (EnvInterpolation)](https://github.com/godaddy/libenclaveapp/blob/main/DESIGN.md#type-2-envinterpolation) +wrapper around `npm` and `npx`. Registry auth tokens are encrypted at rest +under a hardware-bound P-256 key (Secure Enclave / TPM 2.0 / keyring +fallback). At run time the wrapper rewrites `.npmrc` so registry-auth +entries are `${NPM_TOKEN_*}` placeholders, decrypts the bindings in +memory, and `execve()`s the real `npm` / `npx` binary with the decrypted +values in its environment. + +This document names what npmenc does and does not defend against in that +delivery model. Foundational threats shared with other libenclaveapp +consumers are documented in +[libenclaveapp/THREAT_MODEL.md](https://github.com/godaddy/libenclaveapp/blob/main/THREAT_MODEL.md); +this file focuses on the npm-specific layer. + +## Assets + +| Asset | Where it lives | Sensitivity | +|---|---|---| +| Registry auth token (ciphertext) | `$NPMENC_CONFIG_DIR/secrets/`, ECIES under hardware key | Medium — useless without the hardware key | +| Registry auth token (in env) | `execve()`'d `npm` process's environment; `/proc//environ` | **High — readable by same-UID processes for npm's full lifetime** | +| `.npmrc` | Managed form after install uses `${NPM_TOKEN_*}` placeholders; no plaintext tokens | Medium — reveals registry names | +| `bindings.json` | Non-secret metadata (auth keys, labels, registry URLs, provenance) | Low | +| Hardware P-256 private key | Secure Enclave / TPM / keyring | Critical — same as all libenclaveapp apps | + +## Trust boundaries + +| Boundary | Trusted side | Untrusted side | +|---|---|---| +| Hardware key | SE / TPM chip | Everything else | +| Encrypted secret file | Ciphertext bytes on disk (opaque) | Same-user processes that can read the ciphertext | +| Decrypted token in npmenc's RAM | npmenc process (mlock + zeroize around spawn) | Processes that can `ptrace` / read `/proc//mem` | +| `execve()` to npm | npmenc | **npm and every descendant process (lifecycle scripts, node-gyp, helper children)** | +| `.npmrc` on disk | Owning user | Anyone who can read the file (only placeholders after install) | +| Token-source helper subprocess | The helper | npmenc receives its stdout | + +## What npmenc protects + +With a hardware backend: + +- **The encrypted token file is useless on another machine.** ECIES + ciphertext is bound to the SE/TPM on the machine that created the key. +- **Plaintext never touches disk.** `.npmrc` contains only placeholders + after install; the decrypted value lives only in the launcher's memory + and in the child `npm` process's environment. +- **State transitions are atomic.** `.npmrc` rewrites and bindings-store + mutations go through tmp-then-rename (`npmenc-core/src/atomic_write.rs` + for `.npmrc`, `enclaveapp-app-adapter` for bindings). A power loss or + crash mid-operation leaves either the old file or the new file, never + a partial mix, and original mode bits on `.npmrc` are preserved. +- **Direct exec, no shell.** The target `npm` / `npx` binary is launched + via `Command::new(path).env(...)` — no `sh -c`, no shell history, no + env-value splitting into argv. +- **Secret lifecycle is bounded in npmenc's process.** The launcher + `mlock`s env-var bytes and zeroizes them after the child exits. + +## What npmenc cannot protect (Type 2 fundamentals) + +Once `execve()` hands `NPM_TOKEN_*` to npm, the following threats apply +until that child and all its descendants exit. **These are inherent to +Type 2 delivery and cannot be fixed inside npmenc.** They are the +operator's responsibility. + +### Threat: Malicious npm lifecycle script exfiltrates the token + +`npm install` runs `preinstall`, `install`, and `postinstall` scripts for +every transitive dependency. These scripts inherit the `npm` process's +environment — including `NPM_TOKEN_*`. A malicious dep (typosquat, +compromised maintainer, or a dep of a dep) can read +`process.env.NPM_TOKEN_*` and exfiltrate the token. + +**This is the single largest threat specific to npm as a Type 2 target.** + +**Mitigations** (user-side; npmenc cannot enforce them): + +- Prefer granular / short-lived npm automation tokens scoped to publish + rights for specific packages, not long-lived global tokens. +- Use `npm install --ignore-scripts` in untrusted trees, or use lock-file- + driven `npm ci` with known-good trees only. +- Rotate tokens promptly when exposure is suspected. + +### Threat: Same-UID reader of `/proc//environ` + +Any process the user runs while `npm install` is live can read +`/proc//environ` and learn the current `NPM_TOKEN_*` values. +`npm install` can take minutes; the window is large. + +**Mitigations:** none possible at the npmenc layer. `mlock` + zeroize +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. + +### Threat: npm's own logging / telemetry leaks the token + +npm can log, send telemetry, or persist crash reports that include env +vars. Any such leak is outside npmenc's control. + +**Mitigations:** disable npm telemetry / crash reporting at the npm +config level; treat the token as compromised if npm logs full env on +error. + +## npmenc-specific threats (non-fundamental) + +### Threat: Rollback of `.npmrc` to plaintext + +After `npmenc install`, the user's VCS / backup system may still contain +a pre-install `.npmrc` with raw tokens. Restoring from backup or +reverting a commit resurrects plaintext tokens in the working tree. + +**Mitigations:** user-side hygiene. Operators who backed up a plaintext +`.npmrc` should rotate the stashed token after running `npmenc install`. + +### Threat: Partial `.npmrc` rewrite on crash + +`.npmrc` rewrites in `npmenc-core/src/install.rs` and `uninstall.rs` now +use `atomic_write_preserving_mode` — tmp-then-rename with preserved mode +bits. A crash or power loss during rewrite cannot leave a half-stripped +file on disk. + +**Residual risk:** the tmp file in the `.npmrc`'s parent directory lives +briefly (typically milliseconds) during the write. Named with +`tempfile::NamedTempFile` randomization so collisions are not a concern. + +### Threat: Concurrent `npm config set` racing `npmenc install` + +`npm` itself can rewrite `.npmrc` while `npmenc install` is running. The +state lock (`npmenc-core/src/state_lock.rs`) coordinates npmenc vs. +npmenc, not npmenc vs. npm. + +**Residual risk:** last writer wins on `.npmrc`; users should avoid +running `npm config` commands concurrently with `npmenc install` / +`uninstall`. Outcome is data loss at worst, not a security issue. + +### Threat: `auth_key_to_registry_url` reconstructs every URL as `https://` + +`.npmrc` auth keys start with `//` and carry no scheme +(`//registry.npmjs.org/:_authToken=...`). The URL reconstructor in +`npmenc-core/src/registry_bindings.rs` hardcodes `https://`. A user with +an internal `http://` registry will see a wrong reconstructed URL if +they use the auth-key-only code path. + +**Mitigations:** documented in-source as deliberate secure-by-default; +callers are steered toward `RegistryBinding::registry_url`, which +preserves the scheme supplied at registration. + +**Residual risk:** low; wrong URL typically manifests as a connect +failure against an internal HTTP registry, not a token leak. + +### Threat: `NPM_TOKEN_*` injection when it is not needed + +Every `npmenc ` invocation injects `NPM_TOKEN_*` into the target +process's environment. Most npm subcommands (`install`, `ci`, +`run-script`, `publish`) do need the token, but many (`version`, +`init`, local-only scripts) do not. The current design over-injects. + +**Tracked as hardening work.** A `--publish-only` gate that restricts +injection to subcommands that actually authenticate to the registry +(`publish`, `whoami`, `access`, `owner`, `deprecate`, `unpublish`) would +materially reduce the `install`-time exposure window — the most +dangerous because of lifecycle scripts. + +**Residual risk:** full env injection for every subcommand is the +default. Users can avoid injection by running `npm` directly (bypassing +npmenc), but then `.npmrc` placeholders are not resolved. + +### Threat: Token-source subprocess hang + +`npmenc` can fetch tokens from external helpers (sso-jwt, gh CLI, custom +scripts) via `token_source`. A wedged helper would hang the `npmenc` +process indefinitely. + +**Mitigations:** the token-source subprocess acquisition enforces a +30-second timeout (`npmenc-core/src/token_source.rs`) using a reader +thread plus `mpsc::recv_timeout`, matching the sso-jwt `gh` pattern. + +**Residual risk:** a cooperating-but-slow helper can still block up to +the timeout. Legitimate interactive helpers that need to prompt the +user must complete within 30 seconds. + +### Threat: `` sentinel collision + +`enclaveapp-app-adapter::secret_store` returns the literal string +`""` from read-only stores (the `REDACTED_PLACEHOLDER` +constant). If a real npm token were literally the string `` a +consumer comparing against `REDACTED_PLACEHOLDER` would be confused. +npm tokens start with `npm_` by convention so the collision is +theoretical. + +**Residual risk:** negligible for npm. Documented upstream. + +### Threat: Binary planting on `npm` / `npx` resolution + +npmenc resolves the real `npm` / `npx` binary via the adapter's program +resolver. Default mode (`auto`) consults `command -v` in the user's +login shell; `path-only` and an explicit `--npm-bin` flag are available +escape hatches. PATH-inserted lookalikes (or shims installed by asdf / +volta / Volta-style wrappers) are followed. + +**Mitigations:** `--npm-bin=/absolute/path/to/npm` is the hardened +mode. Alias / function resolution has an 8-deep recursion cap. + +**Residual risk:** default mode trusts whatever the user's shell +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: Multi-user machines + +`npmenc` config paths are per-user (`dirs::config_dir()`-based). File +permissions (0700 dir, 0600 files) block user B from reading user A's +ciphertext. + +**Residual risk:** on Windows, `set_dir_permissions` / +`set_file_permissions` in the adapter are no-ops; file ACLs are +inherited from the parent directory. On a well-configured per-user +profile this matches the OS-expected security, but it is not +defense-in-depth against an attacker who has already stolen the user's +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 +[libenclaveapp threat model](https://github.com/godaddy/libenclaveapp/blob/main/THREAT_MODEL.md) +for details. + +## Top residual risks (cannot be fixed inside npmenc) + +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. +2. **Same-UID process reads `/proc//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 + 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. + +## Out of scope + +- Physical attacks on SE / TPM hardware. +- Kernel / hypervisor exploits. +- Server-side issues at the npm registry. +- Supply chain attacks on npmenc's own Rust dependencies (deferred to + [libenclaveapp/THREAT_MODEL.md](https://github.com/godaddy/libenclaveapp/blob/main/THREAT_MODEL.md)). +- Denial of service — an attacker who can delete the config or secret + files can force re-authentication but cannot recover the token. diff --git a/npmenc-core/src/atomic_write.rs b/npmenc-core/src/atomic_write.rs new file mode 100644 index 0000000..736c53c --- /dev/null +++ b/npmenc-core/src/atomic_write.rs @@ -0,0 +1,108 @@ +#![cfg_attr(test, allow(clippy::unwrap_used))] + +//! Atomic file rewrite that preserves the target file's existing mode bits. +//! +//! `.npmrc` rewrites are safety-critical: a partial write caused by power +//! loss or a crash mid-`fs::write` leaves the file in a hybrid state with +//! the raw-token prefix truncated away but the managed placeholder not yet +//! written. This module writes the new contents to a temporary file in +//! the same parent directory and then `rename`s it into place so the +//! transition is observably atomic. + +use std::fs; +use std::io::Write; +use std::path::Path; + +use anyhow::{Context, Result}; + +/// Atomically replace the contents of `path` with `contents`. +/// +/// If `path` already exists, its Unix mode bits are preserved across the +/// replacement so a pre-existing `0600` `.npmrc` does not get silently +/// widened to the default umask. On Windows, platform permissions are +/// inherited from the parent directory (matching `fs::write` behavior). +/// +/// Implementation: write to a sibling temp file with a unique name, flush, +/// sync, copy the original's mode bits onto the temp file (Unix), then +/// `rename` over the target. `rename(2)` is atomic within a single +/// filesystem, so readers either see the old contents or the new contents, +/// never a partial mix. +pub fn atomic_write_preserving_mode(path: &Path, contents: &[u8]) -> Result<()> { + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + + #[cfg(unix)] + let original_mode = fs::metadata(path).ok().map(|meta| { + use std::os::unix::fs::PermissionsExt; + meta.permissions().mode() + }); + + let mut temp = tempfile::NamedTempFile::new_in(parent) + .with_context(|| format!("creating temporary file in {}", parent.display()))?; + temp.write_all(contents) + .with_context(|| format!("writing temporary file for {}", path.display()))?; + temp.flush() + .with_context(|| format!("flushing temporary file for {}", path.display()))?; + temp.as_file() + .sync_all() + .with_context(|| format!("syncing temporary file for {}", path.display()))?; + + #[cfg(unix)] + if let Some(mode) = original_mode { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(temp.path(), fs::Permissions::from_mode(mode)) + .with_context(|| format!("preserving mode on temp for {}", path.display()))?; + } + + temp.persist(path) + .map_err(|e| e.error) + .with_context(|| format!("renaming temp into place at {}", path.display()))?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn writes_new_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("new.txt"); + atomic_write_preserving_mode(&path, b"hello").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"hello"); + } + + #[test] + fn overwrites_existing_file_atomically() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("file.txt"); + fs::write(&path, "first").unwrap(); + atomic_write_preserving_mode(&path, b"second").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"second"); + } + + #[cfg(unix)] + #[test] + fn preserves_existing_mode_bits() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("file.txt"); + fs::write(&path, "first").unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o600)).unwrap(); + atomic_write_preserving_mode(&path, b"second").unwrap(); + let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } + + #[cfg(unix)] + #[test] + fn new_file_gets_default_permissions() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("new.txt"); + atomic_write_preserving_mode(&path, b"hello").unwrap(); + let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777; + // NamedTempFile in tempfile 3.x creates with 0600 on Unix. + assert_eq!(mode, 0o600); + } +} diff --git a/npmenc-core/src/install.rs b/npmenc-core/src/install.rs index 1218e03..7b21b4d 100644 --- a/npmenc-core/src/install.rs +++ b/npmenc-core/src/install.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use enclaveapp_app_adapter::{BindingStore, SecretStore}; +use crate::atomic_write::atomic_write_preserving_mode; use crate::common::restore_previous_secret; use crate::config_path::resolve_effective_userconfig; use crate::management::validate_unique_auth_keys; @@ -352,7 +353,7 @@ where ); let changed = rewritten.contents != source; if changed { - fs::write(&path, &rewritten.contents)?; + atomic_write_preserving_mode(&path, rewritten.contents.as_bytes())?; } if let Err(error) = apply_install_state_changes( &pending_record_updates, @@ -361,7 +362,7 @@ where secret_store, ) { if changed { - if let Err(restore_error) = fs::write(&path, &source) { + if let Err(restore_error) = atomic_write_preserving_mode(&path, source.as_bytes()) { return Err(anyhow::anyhow!( "{error}; additionally failed to restore original config at {}: {restore_error}", path.display() diff --git a/npmenc-core/src/lib.rs b/npmenc-core/src/lib.rs index 7b996f0..15cecde 100644 --- a/npmenc-core/src/lib.rs +++ b/npmenc-core/src/lib.rs @@ -1,3 +1,4 @@ +pub mod atomic_write; pub mod cli_common; pub mod command_kind; pub mod common; diff --git a/npmenc-core/src/uninstall.rs b/npmenc-core/src/uninstall.rs index 93de3b4..e53b1b9 100644 --- a/npmenc-core/src/uninstall.rs +++ b/npmenc-core/src/uninstall.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use enclaveapp_app_adapter::{AdapterError, BindingStore, SecretStore}; +use crate::atomic_write::atomic_write_preserving_mode; use crate::common::restore_previous_secret; use crate::config_path::resolve_effective_userconfig; use crate::management::validate_unique_auth_keys; @@ -101,14 +102,16 @@ where ); let changed = restored != source; if changed { - fs::write(&path, &restored)?; + atomic_write_preserving_mode(&path, restored.as_bytes())?; } let pending_updates = match build_uninstall_updates(&binding_records, &path_string, purge) { Ok(updates) => updates, Err(error) => { if changed { - if let Err(restore_error) = fs::write(&path, &source) { + if let Err(restore_error) = + atomic_write_preserving_mode(&path, source.as_bytes()) + { return Err(anyhow::anyhow!( "{error}; additionally failed to restore original config at {}: {restore_error}", path.display() @@ -125,7 +128,7 @@ where secret_store, ) { if changed { - if let Err(restore_error) = fs::write(&path, &source) { + if let Err(restore_error) = atomic_write_preserving_mode(&path, source.as_bytes()) { return Err(anyhow::anyhow!( "{error}; additionally failed to restore original config at {}: {restore_error}", path.display() From d60db8960e498ca7f86106ce398c255a52f0397f Mon Sep 17 00:00:00 2001 From: Jay Gowdy Date: Thu, 16 Apr 2026 22:16:58 -0700 Subject: [PATCH 2/2] Fix Windows build: gate std::fs to unix paths in atomic_write The module-level `use std::fs;` was flagged as unused on Windows by `-D warnings` because both `fs::metadata` and `fs::set_permissions` live inside `#[cfg(unix)]` blocks. Switch those two call sites to fully-qualified `std::fs::...` paths and move the `use std::fs;` into the tests module where it's consumed on all platforms by `fs::write` / `fs::read` helpers. --- npmenc-core/src/atomic_write.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/npmenc-core/src/atomic_write.rs b/npmenc-core/src/atomic_write.rs index 736c53c..56c300b 100644 --- a/npmenc-core/src/atomic_write.rs +++ b/npmenc-core/src/atomic_write.rs @@ -9,7 +9,6 @@ //! the same parent directory and then `rename`s it into place so the //! transition is observably atomic. -use std::fs; use std::io::Write; use std::path::Path; @@ -31,7 +30,7 @@ pub fn atomic_write_preserving_mode(path: &Path, contents: &[u8]) -> Result<()> let parent = path.parent().unwrap_or_else(|| Path::new(".")); #[cfg(unix)] - let original_mode = fs::metadata(path).ok().map(|meta| { + let original_mode = std::fs::metadata(path).ok().map(|meta| { use std::os::unix::fs::PermissionsExt; meta.permissions().mode() }); @@ -49,7 +48,7 @@ pub fn atomic_write_preserving_mode(path: &Path, contents: &[u8]) -> Result<()> #[cfg(unix)] if let Some(mode) = original_mode { use std::os::unix::fs::PermissionsExt; - fs::set_permissions(temp.path(), fs::Permissions::from_mode(mode)) + std::fs::set_permissions(temp.path(), std::fs::Permissions::from_mode(mode)) .with_context(|| format!("preserving mode on temp for {}", path.display()))?; } @@ -63,6 +62,7 @@ pub fn atomic_write_preserving_mode(path: &Path, contents: &[u8]) -> Result<()> #[cfg(test)] mod tests { use super::*; + use std::fs; #[test] fn writes_new_file() {