feat(THU-448): App version management: track device versions and enforce minimum version (BACKEND)#985
Conversation
- Add app_version column to devices and persist X-App-Version on token issue - Cap header at 32 chars; leave column untouched when header is absent - Surface minAppVersion via GET /config (omitted when unset) for client-side enforcement
arienemaiara
changed the title
Semgrep Security ScanNo security issues found. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 19b1bd2. Configure here.
PR Metrics
Updated Thu, 25 Jun 2026 21:13:46 GMT · run #2057 |
|
Preview environment destroyed 🧹 Stack |
- Server manages devices.app_version via X-App-Version header on requests - Add app_version to PowerSync upload deny list so clients can't override it - Test confirms PATCH strips app_version while letting other fields through
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // Minimum app version clients must run. Empty string disables enforcement. | ||
| // Surfaced to the frontend via GET /config; clients below this hard-block until they update. | ||
| minAppVersion: z.string().default(''), |
There was a problem hiding this comment.
Would it make sense to validate that MIN_APP_VERSION is actually semver before the app boots? Right now a typo like 0,2,0 or banana starts up fine and gets served to every client, where the frontend will then choke trying to compare it. Since this file already uses superRefine for the PowerSync secret check, a quick regex guard there would fail fast at startup and save operators some painful debugging down the line.
| builtInAgentEnabled: !settings.disableBuiltInAgent, | ||
| allowCustomAgents: settings.allowCustomAgents, | ||
| // Omit when unset so the frontend treats it as "no enforcement" without parsing an empty string as semver. | ||
| minAppVersion: settings.minAppVersion || undefined, |
There was a problem hiding this comment.
The || undefined correctly omits empty strings, but a whitespace only value like MIN_APP_VERSION=' ' would slip through as truthy and reach every client as a padded string. Trimming either here or in the settings parser would harden this nicely. No test covers this edge case today either.
| rawDeviceName && rawDeviceName.length > 0 && rawDeviceName.length <= 100 ? rawDeviceName : 'Unknown device' | ||
| const rawAppVersion = request.headers.get('x-app-version')?.trim() | ||
| // Cap to a sane length so a malformed/oversized header never bloats the row. | ||
| const appVersion = rawAppVersion && rawAppVersion.length > 0 && rawAppVersion.length <= 32 ? rawAppVersion : undefined |
There was a problem hiding this comment.
One thing worth a quick thought: a full semver with prerelease and build metadata can exceed 32 characters (something like 1.234.567-rc.1+build.2024.01.01 is 35). When that happens the value gets silently dropped and the prior version is kept, which could confuse operators looking at device rows. If rich semver is on the roadmap for the frontend, bumping the cap or documenting why 32 was chosen would help future readers. Totally fine to defer if build metadata is never expected.
- Raise X-App-Version length cap from 32 to 64 to fit full semver with prerelease + build metadata (e.g. 1.234.567-rc.1+build.x). - Trim and semver-validate MIN_APP_VERSION at startup so typos like "banana" or "0,2,0" fail fast instead of reaching clients and breaking version comparison.
ital0
left a comment
ital0
left a comment
There was a problem hiding this comment.
Re-reviewed after the fixes and this all looks great. A few small optional notes below, nothing blocking.
| lastSeen: timestamp('last_seen').defaultNow(), | ||
| createdAt: timestamp('created_at').defaultNow(), | ||
| revokedAt: timestamp('revoked_at'), | ||
| appVersion: text('app_version'), |
There was a problem hiding this comment.
Quick confirm: since devices syncs SELECT *, app_version reaches clients but isn't declared in the frontend schema (src/db/tables.ts), so PowerSync just keeps it internally. Reads as a deliberate server side field, all good. If you ever want it in the device list UI, the follow up is adding appVersion to the frontend devicesTable.
There was a problem hiding this comment.
Confirmed — that's the intent. app_version is server-managed (written from X-App-Version on /v1/token, denied on PowerSync uploads via uploadDenyColumns), so it lives on the synced devices table for operator visibility without needing client-side reads. Surfacing it in the device list UI is a natural follow-up: declare appVersion on src/db/tables.ts's devicesTable and it'll start landing in the SQLite store. Happy to file a ticket if useful.
| builtInAgentEnabled: !settings.disableBuiltInAgent, | ||
| allowCustomAgents: settings.allowCustomAgents, | ||
| // Omit when unset so the frontend treats it as "no enforcement" without parsing an empty string as semver. | ||
| minAppVersion: settings.minAppVersion || undefined, |
There was a problem hiding this comment.
Confirming intent: enforcement is client side only (the upgrade screen), and the backend still issues sync tokens and accepts uploads from clients below the floor. Reads as a UX nudge by design, so fine. If old clients ever need a hard block, that check would live on /powersync/token.
There was a problem hiding this comment.
Confirmed — client-side UX nudge by design. The original ticket scoped this as "hard-block the app with a non-dismissable modal/screen … no access to chat, settings, or sync until they upgrade," which the upgrade screen + provider tree short-circuit covers from the user's side. Backend-side hard block on /v1/token (and likely /v1/powersync/upload) is the right place if/when we want server-enforced refusal — happy to file a follow-up if that becomes a requirement.
2 participants
Note
Medium Risk
Adds a nullable DB column and changes PowerSync token/device upsert behavior; misconfigured MIN_APP_VERSION fails startup but enforcement impact depends on the frontend companion change.
Overview
Adds minimum app version enforcement plumbing on the backend:
MIN_APP_VERSIONis read from the environment, validated as semver at startup, and returned from public GET/configasminAppVersion(omitted when unset so clients treat it as no enforcement).Devices gain an
app_versioncolumn (migration0020). On PowerSync token issuance, the server readsX-App-Version(trimmed, max 64 chars), stores it viaupsertDevice, and ignores oversized values. Clients cannot setapp_versionthrough PowerSync upload PATCH—it is listed in upload deny columns alongside trust/crypto fields.Tests cover config exposure, settings validation, token persistence, header omission, long header rejection, and upload stripping.
Reviewed by Cursor Bugbot for commit 240d0c7. Bugbot is set up for automated code reviews on this repo. Configure here.