Skip to content

fix: privacy and session-integrity hardening across auth, SOS, photos, and telemedicine - #1004

Merged
llinsss merged 6 commits into
DogStark:mainfrom
Calebux:fix/privacy-consent-and-session-race-hardening
Aug 28, 2026
Merged

fix: privacy and session-integrity hardening across auth, SOS, photos, and telemedicine#1004
llinsss merged 6 commits into
DogStark:mainfrom
Calebux:fix/privacy-consent-and-session-race-hardening

Conversation

@Calebux

@Calebux Calebux commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Four privacy and session-integrity fixes, each with regression tests that fail against the current main.

Closes #904
Closes #942
Closes #963
Closes #969

On scope: each issue asks for a narrowly scoped PR, and these touch four unrelated subsystems. They were bundled at the requester's direction. Every issue is a self-contained commit, so this splits cleanly if you'd prefer four PRs — say the word.


#904 — Logout races restoring refreshed credentials

refreshToken() awaited a network round-trip, then wrote the response with storeSecureTokens(). When logout() ran inside that window the write landed after clearSecureTokens(), silently restoring the session the user had just ended.

Three variants of the same race:

Scenario Before
Refresh in flight → logout Tokens restored after logout
Refresh in flight → sign in as B Account A's tokens overwrite B's
Slow refresh fails → new session started New session's tokens cleared

Fix. Every credential write is tagged with a monotonic session generation. logout, login, register, loginWithOAuth and revokeOAuthToken bump it; a refresh resolving against a superseded generation discards its response and rejects with SESSION_CHANGED. The generation is re-checked after storeSecureTokens(), since that call is itself async.

Concurrent callers now share one in-flight refresh instead of each firing their own — several screens calling getToken() at once previously raced to write.

The staleness check is structural rather than instanceof AuthError: subclassing Error doesn't survive Babel's class transform, so instanceof can be false for an error this module threw itself. That cost me a debugging cycle and is worth knowing about elsewhere in the codebase.

#942 — Offline SOS leaking to unintended recipients

Two disclosures nobody chose:

Public hotlines were auto-messaged. DEFAULT_CONTACTS seeds the Pet Poison Helpline and ASPCA Animal Poison Control on first launch. sendSOSAlerts texted contacts[0]; triggerSOS auto-dialled the first 24h contact. On a fresh install both resolved to the helpline — so a first-run SOS sent the owner's precise GPS coordinates to a national hotline and called it.

Coordinates were fabricated. With no GPS fix the location fell back to { latitude: 0, longitude: 0 } — a real point in the Gulf of Guinea, indistinguishable from a true fix to whoever receives it.

Fix. prepareSOS() returns the exact dispatch plan — recipients, the verbatim message body, resolved location, connectivity, channels, warnings — so the UI can show it before anything is sent. SOSButton renders it during the existing 3-second countdown, so the user sees who will be contacted and can cancel.

Only user-curated contacts are eligible: explicitly confirmed ids, else their own favourites. Bundled hotlines are never auto-messaged. Location is null rather than faked, omitted from the session request, and the message says so outright. Offline, the network call is skipped entirely (it previously burned the full timeout) and call/SMS still go out.

Also fixes an aliasing bug this surfaced: getEmergencyContacts() returned the module-level DEFAULT_CONTACTS array by reference, and addContact() pushed straight into it — permanently corrupting the seed list for the rest of the process. Now frozen and returned as a copy.

#963 — EXIF stripping

The JPEG re-encode already dropped metadata; the pipeline around it didn't hold up.

  • resize: { width: maxDim } ran unconditionally, upscaling a 400px photo to 1280px — bigger upload, worse quality.
  • Portrait images were constrained by width, leaving the long edge oversized.
  • Two passes encoded at quality 1.0 then re-encoded, compounding artefacts for nothing.
  • Nothing verified the result, so a failed resize or zero-dimension output reached the CDN.
  • No size or format limit existed, despite the issue asking for one.

Fix. Resize only when the longest edge exceeds the preset maximum, constrained by whichever edge is longer; encode once; verify before upload. verifyProcessedOutput asserts the aspect ratio survived — that's what catches a decoder ignoring the EXIF orientation tag and turning a portrait photo landscape. PHOTO_LIMITS documents and enforces the input size cap and accepted containers, checked against the picker's reported file size before decode.

#969 — Telemedicine consent

One dialog covered camera, microphone, screen and recording, and recorded almost nothing:

  • The POST body was empty — the backend couldn't tell who consented.
  • Only "I Consent" posted; a decline was indistinguishable from no answer.
  • No policy version, so re-wording the prompt silently kept old consent valid.
  • getUserMedia ran independently of the answer, opening the camera before the user knew what the call involved.
  • Screen sharing had no consent at all, despite exposing anything on the device.
  • The end screen claimed "Your session recording has been saved securely" whenever consent was given — though consent alone never started a recording.
  • The fetch carried no auth headers.

Fix. New consultationConsentService with per-scope decisions (camera, microphone, recording, screen_share, file_upload), each attributed to a participant and role, stamped with CONSENT_POLICY_VERSION and an ISO timestamp, posted through apiClient so it carries auth. Denials are recorded as explicitly as grants, and the local record is written first and unconditionally so a network failure can't make a denial look like "never asked".

hasCurrentConsent returns true only for a grant under the current policy version — bump the constant and participants are re-asked. Consent is per-consultation and cleared when the call ends.


Included: a one-line fix that unblocks CI

src/utils/sanitize.ts documented the SQL meta-characters it strips and included a literal */ in the list. That closed the JSDoc block early, so everything after it parsed as code — 11 TypeScript syntax errors from one line.

tsc bailed on the parse failure, so "Lint, Format & Typecheck" failed on every branch and "Unit & Integration Tests" — which depends on it — was skipped rather than run. That's why CI is currently red repo-wide.

Included here because authService.ts imports sanitizeString, so #904 sits directly downstream of it. The rest of that file's diff is prettier's own output — the file was previously unparseable, so format:check never reached it.

Verification

Gate Result
New tests 52 passing across 4 suites
Fail against current main Confirmed for #904 (5 of 7 fail)
tsc --noEmit 17 errors before, 17 after — none in changed files
eslint on changed files 0 errors
prettier --check on changed files clean

Two things you should know

The test suite cannot execute on main. jest.setup.js imports msw/node, which pulls rettime — an ESM-only .mjs package that Jest's CJS runtime refuses to require(). It fails before any test runs. I verified my suites against a local config that omits the msw setup and changed no committed Jest config, since fixing msw/Jest interop is its own piece of work with real blast radius. Happy to open a separate issue or PR for it.

src/services/__tests__/authService.test.ts has never run. Its jest.mock() factories reference out-of-scope variables (biometricAuthShouldFail, keychainStore, supportedBiometryType), which Jest rejects outright. Untouched here, but it's dead coverage — worth its own fix.

Both are pre-existing and out of scope; flagging them so the CI state isn't mistaken for something this PR introduced.

Platform notes

All four changes are in TypeScript service and screen logic with no native module changes, so iOS/Android behaviour is identical. Two platform-specific paths are covered by tests: SMS URL separator (& on iOS, ? on Android, pre-existing) and extensionless content:// URIs from the Android picker, which PHOTO_LIMITS deliberately allows through since the decoder is the real authority on the bytes.

No screenshots included — the visible changes are the SOS recipient preview and the reworded consent dialog, neither of which I can capture without a simulator in this environment. Both are plain <Text> in existing containers.

🤖 Generated with Claude Code

Caleb and others added 5 commits August 25, 2026 19:27
The doc comment for `sanitizeString` listed the SQL meta-characters it strips
and included a literal `*/` among them. That sequence closed the block comment
early, so everything after it was parsed as code — 11 TypeScript syntax errors
cascading from one line.

Because `tsc` bailed on the parse failure, the "Lint, Format & Typecheck" CI job
failed on every branch, and the "Unit & Integration Tests" job — which depends
on it — was skipped rather than run.

Reworded the list to describe the delimiters instead of quoting them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`refreshToken()` awaited a network round-trip and then wrote the response with
`storeSecureTokens()`. If `logout()` ran during that window the write landed
after `clearSecureTokens()`, silently restoring the session the user had just
ended. The same race restored the previous account's tokens on account switch,
and a failed stale refresh could clear the tokens of a session that started
after it.

Tags every credential write with a monotonic session generation. `logout()`,
`login()`, `register()`, `loginWithOAuth()` and `revokeOAuthToken()` bump it;
a refresh that resolves against a superseded generation discards its response
and rejects with `SESSION_CHANGED` instead of writing. The generation is
re-checked after `storeSecureTokens()` too, since that call is itself async.

Concurrent callers now share a single in-flight refresh rather than each firing
their own, so several screens calling `getToken()` at once no longer race each
other to write.

The stale-session check is structural rather than `instanceof AuthError`:
subclassing `Error` does not survive Babel's class transform, so `instanceof`
can be false for an error this module threw itself.

Adds 7 tests that drive the race deterministically with a deferred response.
They fail against the previous implementation.

Closes DogStark#904

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d recipients

Two disclosures went out that nobody chose.

`DEFAULT_CONTACTS` seeds two public poison-control hotlines on first launch,
and `sendSOSAlerts` texted `contacts[0]` while `triggerSOS` auto-dialled the
first 24h contact. On a fresh install both resolved to the Pet Poison Helpline,
so a first-run SOS sent the owner's precise coordinates to a national hotline.

When GPS was unavailable the location fell back to `{ latitude: 0, longitude: 0 }`
— a real point in the Gulf of Guinea that a responder cannot distinguish from a
true fix. Offline, the live-session POST also burned its full timeout before
falling through.

Adds `prepareSOS()`, which returns the exact dispatch plan — recipients, the
verbatim message body, resolved location, connectivity, channels and warnings —
so the UI can show it before anything is sent. `SOSButton` renders that plan
during its existing 3-second countdown, so the user sees who will be contacted
and can cancel.

Only contacts the user curated are eligible: explicitly confirmed ids, else
their own favourites. Bundled hotlines are never auto-messaged. Location is
`null` rather than fabricated when unknown, omitted from the session request,
and the message says so explicitly. Offline the network call is skipped
entirely and call/SMS still go out.

Also fixes a pre-existing aliasing bug this surfaced: `getEmergencyContacts()`
returned the module-level `DEFAULT_CONTACTS` array by reference and
`addContact()` pushed into it, permanently corrupting the defaults for the rest
of the process. The seed list is now frozen and returned as a copy.

Adds 13 tests covering airplane mode, absent GPS and recipient selection.

Closes DogStark#942

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The JPEG re-encode already dropped EXIF, but the pipeline around it did not
hold up:

- `resize: { width: maxDim }` ran unconditionally, so a 400px photo was
  upscaled to 1280px — a bigger upload at worse quality.
- Portrait images were constrained by width, leaving the long edge oversized.
- Two passes encoded to JPEG at quality 1.0 and then re-encoded, compounding
  artefacts for no benefit.
- Nothing checked the result, so a failed resize or a zero-dimension output
  would reach the CDN unnoticed.
- No size or format limit existed anywhere, despite the issue asking for one.

Now resizes only when the longest edge exceeds the preset maximum, constrains
by whichever edge is longer, encodes once, and verifies the output before the
upload starts. `verifyProcessedOutput` also asserts the aspect ratio survived,
which is what catches a decoder that ignored the EXIF orientation tag and
turned a portrait photo landscape.

`PHOTO_LIMITS` documents and enforces the maximum input size and the accepted
source containers; the screen passes the picker's reported file size so an
oversized file is rejected before decode rather than after a long upload.

Adds 20 tests.

Closes DogStark#963

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…and capture

A consultation can reach the camera, the microphone, the screen and files, and
the session may be written into the pet's medical record. All of that was
covered by one dialog that recorded almost nothing:

- The POST body was empty, so the backend could not tell who consented.
- Only "I Consent" posted at all — a decline was indistinguishable from a user
  who never answered.
- No policy version, so re-wording the prompt silently kept old consent valid.
- `getUserMedia` ran independently of the answer, opening the camera and
  microphone before the user knew what the call involved.
- Screen sharing had no consent whatsoever, despite exposing anything on the
  device to the vet.
- The end screen claimed "Your session recording has been saved securely"
  whenever consent was given, even though consent alone never started one.
- The `fetch` carried no auth headers.

Adds `consultationConsentService`: per-scope decisions (camera, microphone,
recording, screen_share, file_upload), each attributed to a participant and
role, stamped with `CONSENT_POLICY_VERSION` and an ISO timestamp, and posted
through `apiClient` so it carries auth. Denials are recorded as explicitly as
grants. The local record is written first and unconditionally, so a network
failure cannot make a denial look like "never asked".

`hasCurrentConsent` returns true only for a grant under the current policy
version, so bumping the version re-asks. Consent is in-memory and per
consultation, cleared when the call ends.

The screen no longer touches the camera until the participant answers, records
an OS-level permission denial as a denial, asks separately before screen
sharing, and states plainly whether the session was recorded.

Adds 12 tests.

Closes DogStark#969

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@drips-wave

drips-wave Bot commented Aug 25, 2026

Copy link
Copy Markdown

@Calebux Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@llinsss
llinsss merged commit 0220614 into DogStark:main Aug 28, 2026
5 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants