feat(ios): support explicit iOS simulator keychain reset - #2345
Open
thymikee wants to merge 2 commits into
Open
Conversation
`settings clear-app-state` never touched keychain-backed credentials (e.g. Firebase auth), so a customer's fresh-install reset via the CLI left an app signed in when their in-app reset button did not (#2282). simctl exposes no per-app keychain reset, only a whole-simulator one (`simctl keychain <device> reset`), so this ships as a separate, explicit `settings reset-keychain clear` command rather than folding it into `clear-app-state` — callers opt in knowing the scope is the whole simulator, not just the app under test. Split the pre-existing `apps.test.ts` and `snapshot-handler.test.ts` suites along the `app-settings.ts`/`snapshot-settings.ts` modules they actually mirror, since both were already over the test-file-size tripwire and could not grow further.
|
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
Please reject app-scoped input before resetting anything. The CLI accepts settings reset-keychain clear com.example.app and drops the extra argument; the direct daemon parser also accepts it. That can turn an apparent per-app reset into a whole-Simulator keychain wipe. Require exactly reset-keychain clear, and add CLI and direct-daemon tests proving an extra app argument causes no settings mutation. The remaining validation from #2282 is also needed: authenticate with keychain-backed credentials, reset, relaunch, and verify the app starts signed out. The reported disposable-Simulator command success proves invocation, not that login outcome. Reviewed at 3237128. |
…eychain fixture settings reset-keychain clear <extra-arg> silently dropped the extra argument in both the CLI reader and the direct-daemon parser, so a caller expecting per-app scoping could get a whole-simulator wipe without any signal something was off. Reject it instead in both places, with tests proving no settings mutation happens. Also add a small keychain-backed "auth" fixture to the test-app's automation lab (expo-secure-store) so the settings reset-keychain guarantee has a real regression surface: authenticate, verify the credential survives clear-app-state and a plain relaunch, then verify reset-keychain actually clears it. Validated live against a disposable iOS simulator.
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
1 participant
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
settings clear-app-stateclears the app's data container but never touched Keychain, so a customer's fresh-install reset left the app signed in via Firebase auth even though their in-app "Reset to Fresh Install" button cleared it.settings reset-keychain clear, a new iOS-simulator-only settings action that runsxcrun simctl keychain <device> reset.clear-app-state: simctl has no per-app keychain reset, only a whole-simulator one, so folding it into the existing per-app command would silently widen its blast radius.reset-keychaindocuments and requires an explicit opt-in to that wider scope instead.settingsalready dispatches generically to the platform'ssetSettingoperation, so no daemon wire changes were needed beyond the shared contracts/usage-string updates.website/docs/docs/commands.mdand in the CLI's own--helptext, including the "pair withclear-app-statefor a full fresh-install reset" guidance the issue asked for.apps.test.tsandsnapshot-handler.test.tssuites along theapp-settings.ts/snapshot-settings.tsmodules they actually test — both were already over the repo's test-file-size tripwire and could not grow to hold the new tests.Test plan
pnpm typecheckpnpm lintpnpm test:unit(full suite, 9061 passed)agent-device settings reset-keychain clear --udid <udid>against it — succeeded and returned the expected{ scope: 'simulator', cleared: true }payload; confirmed an invalid state (nope) is rejected before touching the device. Disposable simulator deleted afterward.