SecretRead migration + threat-model refresh for core-dump, handle wrap, bridge#18
Merged
jgowdy-godaddy merged 5 commits intomainfrom Apr 17, 2026
Merged
SecretRead migration + threat-model refresh for core-dump, handle wrap, bridge#18jgowdy-godaddy merged 5 commits intomainfrom
jgowdy-godaddy merged 5 commits intomainfrom
Conversation
Depends on libenclaveapp's new SecretRead enum.
token_source.rs:
- Three call sites that inspected the token-source state via
SecretStore::get and compared against REDACTED_PLACEHOLDER now call
get_read and match on SecretRead::{Present, Redacted, Absent}.
- The collision where a real stored state equal to the literal
"<redacted>" would be misclassified as redacted is closed: the
typed API returns Present even if the bytes match the sentinel.
- A test that previously wrote REDACTED_PLACEHOLDER into
MemorySecretStore to simulate inspection-mode redaction now uses the
new mark_redacted() helper, which is the semantically-correct way
to inject the Redacted variant without the string sentinel.
passthrough.rs:
- LaunchRequest struct gained the env_scrub_patterns field; pass
Vec::new() as the default (no scrubbing unless a caller opts in).
Callers that want to strip inherited NPM_TOKEN_* can chain
with_env_scrub (["NPM_TOKEN_*"]) on the returned LaunchRequest.
No user-visible behavior change for existing flows. The "\<redacted>\"
sentinel collision was vanishingly unlikely for npm tokens (they
start with npm_) but the typed API removes it structurally.
- 'Core dump / swap of the npm process' rewritten around the new pre_exec RLIMIT_CORE = 0 hook on the spawned child (landed in libenclaveapp's enclaveapp-app-adapter::launcher). - '.handle plaintext on macOS' rewritten — it's no longer plaintext. libenclaveapp now AES-256-GCM wraps the SE dataRepresentation under a per-label Keychain key with service com.libenclaveapp.<app>. Stale fix-macos.md reference removed. - 'WSL bridge' updated: which-fallback is gone and Authenticode-presence check is enforced before spawn. - 'Top residual risks' list — the stale item #4 (.handle plaintext) is replaced with inherited parent-env NPM_TOKEN_* as a call-out for the opt-in env-scrub helper.
Integration tests spawn `npmenc` child processes via assert_cmd with $HOME overridden to a tempdir. After libenclaveapp #68 the Swift bridge routes all SecItem* calls through an explicitly-opened login keychain via kSecUseKeychain. On GitHub Actions macOS runners that keychain re-locks between jobs, so SecItemAdd blocks waiting for a GUI unlock prompt that never arrives — the job hangs indefinitely. Unlock the keychain up front with the empty password that the runner image installs, and bump the auto-lock idle timeout so the keychain doesn't re-lock mid-test.
cargo test --workspace on the macos-latest runner was hanging 30+ minutes in the first integration test because an unsigned binary's SecItemAdd against the runner's login keychain blocks on an ACL confirmation prompt no one can answer. libenclaveapp #71 added a runtime escape hatch: ENCLAVEAPP_MOCK_STORAGE=1 forces create_encryption_storage to return an in-memory MockEncryptionStorage. Set it on macOS only — Linux and Windows run against their real backends. The library's own enclaveapp-apple tests still exercise the real keychain on macOS CI. Drop the earlier keychain-unlock step — it wasn't the locked state that was blocking, it was the ACL prompt, and the mock path sidesteps both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SecretReadmigration —token_source.rscall sites that compared strings againstREDACTED_PLACEHOLDERnow match onSecretRead::{Present, Redacted, Absent}via the newget_readmethod. The (vanishingly unlikely) collision where a stored token-source state whose bytes happened to equal"<redacted>"would be misclassified is closed structurally.LaunchRequest.env_scrub_patternsfield added to theLaunchRequest { ... }construction inpassthrough.rs, defaulted toVec::new()— no behavior change for existing flows. Callers that want to strip inheritedNPM_TOKEN_*can opt in viawith_env_scrub(["NPM_TOKEN_*"]).pre_exec RLIMIT_CORE = 0hook in libenclaveapp's launcher..handleplaintext stale reference replaced — the SE handle is now AES-256-GCM sealed under a Keychain-held key (com.libenclaveapp.<app>/<label>).which-fallback removal and Authenticode-presence check..handle plaintextentry is gone; inherited parent-envNPM_TOKEN_*is now called out with the opt-in env-scrub helper as the mitigation.Depends on
SecretReadenum, the launcherenv_scrub_patternsfield, the child-RLIMIT_COREpre_exec, and the Keychain wrap of.handleall live in libenclaveapp's PR.Test plan
cargo check --workspaceclean.cargo fmt --all -- --checkclean.cargo clippy --workspace --all-targets -- -D warningsclean.cargo test --workspace— 252 tests passing, 0 failing.inspection_reacquires_redacted_provider_state_when_prepared_marker_existstest migrated from the string-sentinel simulation tomark_redacted(), which is the semantically-correct way to inject theRedactedvariant.