diff --git a/docs/api-method-changes-2026-08-06.md b/docs/api-method-changes-2026-08-06.md new file mode 100644 index 00000000..266f83b4 --- /dev/null +++ b/docs/api-method-changes-2026-08-06.md @@ -0,0 +1,135 @@ +# API method changes — 2026-08-06 + +**For the mobile team.** Five endpoints changed from `POST` to `GET`. **One of them affects the mobile +app**; the other four have no mobile caller. This is a breaking change with no compatibility shim — see +*What breaks and when* below. + +--- + +## Why + +Officer is growing a permission model. Members will get access to specific apps, and within an app the +default is **read everything, write only your own data**. The rule that separates those two is the HTTP +method: safe methods (`GET`, `HEAD`) are reads, everything else is a write. + +That only works if the method tells the truth. These five endpoints read something and returned it, +while announcing themselves as writes — so under the new model a member would be denied a read they are +entitled to, for no reason other than a habit in how the route was declared. + +Nothing else about them changed: same path, same response shape, same auth. + +--- + +## The changes + +| # | Endpoint | Was | Now | Mobile affected | +|---|----------|-----|-----|-----------------| +| 1 | `/api/file-browser/video-info` | `POST` body `{url}` | `GET ?url=` | no | +| 2 | `/api/file-browser/video-playlist` | `POST` body `{url}` | `GET ?url=` | no | +| 3 | `/api/server-settings/ocr/models` | `POST` body `{url}` | `GET ?url=` | no | +| 4 | `/api/transmission/_officer/port-test` | `POST` no body | `GET` | **YES** | +| 5 | `/api/jellyfin/_config/:id/test` | `POST` body `{}` | `GET` | no | + +Note on #5: it already accepted `GET` as well as `POST`. It is now `GET` only, so that the method is a +reliable signal rather than "whichever the caller felt like". + +--- + +## What the mobile app has to change + +**One line.** `packages/core/src/services/transmission.ts:126` + +```ts +// before +export const transmissionPortTest = () => + request<{ open: boolean }>(`${T}/port-test`, { method: 'POST' }); + +// after +export const transmissionPortTest = () => + request<{ open: boolean }>(`${T}/port-test`); +``` + +The response is unchanged: `{ open: boolean }`. + +I searched `monorepo-mobile/packages` and `monorepo-mobile/apps` for callers of the other four and +found none. If you know of one outside those trees, it needs the same treatment — path and response are +identical, only the method and the location of `url` move. + +For the three that take a `url`, it moves from the JSON body to a query parameter and **must be +percent-encoded**: + +```ts +`${base}/file-browser/video-info?url=${encodeURIComponent(url)}` +``` + +--- + +## What breaks and when + +`port-test` returns **405** to a `POST` from the moment the platform is deployed. There is deliberately +no transitional shim accepting both — the whole point of the change is that the method means something, +and an endpoint answering to both methods means nothing. + +The blast radius is one button in the Transmission screen ("test peer port"). It does not affect +torrents, downloads, or anything else in that app. If that is still unacceptable timing, a shim is a +two-line change on the platform side — ask and it can go in, with a date for removal. + +--- + +## What did NOT change, and will not + +Several endpoints look like the ones above but are staying `POST` **on purpose**. If you are tempted to +"fix" them for consistency, please don't — both reasons below are deliberate. + +**They need a request body that `GET` cannot carry:** + +- `POST /api/chat/stt` — multipart audio upload +- `POST /api/file-browser/tts`, `/tts-text`, `/ocr`, `/transcribe` — payloads to transform + +**They carry a credential, and a query string is the wrong place for one.** Query strings are written to +access logs, shell history, proxy logs and `Referer` headers; request bodies are not: + +- `POST /api/server-settings/tts/voices` — takes an `apiKey` +- `POST /api/server-settings/{smtp,tts,stt,ocr}/test` — take connection secrets +- `POST /api/server-settings/local-providers/probe` — takes `{url, auth}` + +These are reads that must remain `POST`. The permission model handles them with an explicit annotation +rather than by inferring from the method, which is why the method change stops at five endpoints instead +of sweeping the whole API. + +Everything else that is `POST`/`PUT`/`PATCH`/`DELETE` genuinely mutates something and is unaffected. + +--- + +## Verifying + +Against a running platform, with a valid token: + +```bash +# should be 200 +curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" \ + "$BASE/api/transmission/_officer/port-test" + +# should be 405 +curl -s -o /dev/null -w '%{http_code}\n' -X POST -H "Authorization: Bearer $TOKEN" \ + "$BASE/api/transmission/_officer/port-test" +``` + +--- + +## Platform-side changes, for reference + +Backend: +- `src/servers/api/file-browser/router.ts` — `/video-info`, `/video-playlist` +- `src/servers/api/server-settings/ocr.ts` — `/models` +- `src/servers/sidecar/transmission/routes.ts` — `handlePortTest` +- `src/servers/sidecar/jellyfin/config.ts` — the `test` action + +Web callers, already updated: +- `src/workspaces/officerdev/src/hooks/useFilesAPI.ts` +- `src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/OCRSection.tsx` +- `src/workspaces/officerdev/src/apps/Transmission/useTransmissionData.ts` +- `src/workspaces/officerdev/src/apps/Jellyfin/useJellyfinData.ts` + +The sidecar contract comments at the top of `transmission/index.ts` and `jellyfin/index.ts` were updated +to match. diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/OCRSection.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/OCRSection.tsx index 277a2721..aa0360ed 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/OCRSection.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/OCRSection.tsx @@ -32,7 +32,9 @@ export const OCRSection = () => { const fetchModels = async (u: string) => { setModelsLoading(true); try { - const res = await client.post<{ models?: string[]; error?: string }>('/server-settings/ocr/models', { url: u }); + const res = await client.get<{ models?: string[]; error?: string }>( + `/server-settings/ocr/models?url=${encodeURIComponent(u)}`, + ); if (res.models) setModels(res.models); else setModels([]); } catch { @@ -111,7 +113,9 @@ export const OCRSection = () => { className="p-0.5 rounded hover:bg-duck-dark/10 dark:hover:bg-foreground/10 cursor-pointer transition-colors disabled:opacity-40" title="Refresh models" > - + {models.length > 0 ? ( @@ -121,7 +125,9 @@ export const OCRSection = () => { {models.map((m) => ( - {m} + + {m} + ))} diff --git a/src/servers/api/file-browser/router.ts b/src/servers/api/file-browser/router.ts index c54b4778..b8dcfaaf 100644 --- a/src/servers/api/file-browser/router.ts +++ b/src/servers/api/file-browser/router.ts @@ -1331,8 +1331,10 @@ router.get('/download-video/:jobId', (ctx) => { // Prefetch a single video's metadata (title / thumbnail / duration / uploader) — proxied straight to // ReClip's /api/info so the download dialog can show a preview card before committing. Errors (private // video, timeout, …) come back as { error } with a 200 so the client can render them inline. -router.post('/video-info', async (ctx) => { - const { url } = ctx.get('body') as { url?: string }; +// GET, not POST: it reads and writes nothing. The method is not cosmetic — the permission model reads +// it to decide whether a non-owner may call this, and a read filed as a write is a read they lose. +router.get('/video-info', async (ctx) => { + const url = ctx.req.query('url'); if (!url) throw errors.BAD_REQUEST('url is required'); const res = await fetch(`${RECLIP_BASE}/api/info`, { method: 'POST', @@ -1347,8 +1349,9 @@ router.post('/video-info', async (ctx) => { // Expand a playlist URL into its individual video URLs (ReClip's /api/playlist → { urls }). The client // then prefetches /video-info per url to build the per-entry cards. -router.post('/video-playlist', async (ctx) => { - const { url } = ctx.get('body') as { url?: string }; +// GET for the same reason as /video-info above. +router.get('/video-playlist', async (ctx) => { + const url = ctx.req.query('url'); if (!url) throw errors.BAD_REQUEST('url is required'); const res = await fetch(`${RECLIP_BASE}/api/playlist`, { method: 'POST', diff --git a/src/servers/api/server-settings/ocr.ts b/src/servers/api/server-settings/ocr.ts index 671d5148..6608b44b 100644 --- a/src/servers/api/server-settings/ocr.ts +++ b/src/servers/api/server-settings/ocr.ts @@ -27,8 +27,11 @@ ocrRouter.put('/', async (ctx) => { return ctx.json({ success: true }); }); -ocrRouter.post('/models', async (ctx) => { - const body = await ctx.req.json<{ url: string }>(); +// GET: it asks a provider what models it has and returns the answer. Nothing is written, and the only +// input is a URL — no credential, so a query string is the right place for it. Kept POST until +// 2026-08-06 purely by habit, and the permission model reads the method. +ocrRouter.get('/models', async (ctx) => { + const body = { url: ctx.req.query('url') ?? '' }; if (!body.url) return ctx.json({ error: 'URL required' }, 400); try { diff --git a/src/servers/sidecar/jellyfin/config.ts b/src/servers/sidecar/jellyfin/config.ts index fe292cb6..f5902b1c 100644 --- a/src/servers/sidecar/jellyfin/config.ts +++ b/src/servers/sidecar/jellyfin/config.ts @@ -226,8 +226,11 @@ export async function handleConfigRoute(req: Request, userId: number, subpath: s return serverList(userId); } + // GET only. It probes one server and reports whether it answered — nothing is written, and nothing is + // switched (that is `activate`, below). It accepted POST as well until 2026-08-06, which made the + // method meaningless for deciding whether this is a read; the permission model needs that answer. if (action === 'test') { - if (req.method !== 'POST' && req.method !== 'GET') return bad('method not allowed', 405); + if (req.method !== 'GET') return bad('method not allowed', 405); return testServer(userId, id); } diff --git a/src/servers/sidecar/jellyfin/index.ts b/src/servers/sidecar/jellyfin/index.ts index 86b4323b..3d28c22b 100644 --- a/src/servers/sidecar/jellyfin/index.ts +++ b/src/servers/sidecar/jellyfin/index.ts @@ -21,7 +21,7 @@ import { getConfig, probe } from './upstream'; // for an access token and never stored // PATCH /_config/:id same fields, all optional; no password keeps the stored token // POST /_config/:id/activate switch to that server -// POST /_config/:id/test probe one server without switching to it +// GET /_config/:id/test probe one server without switching to it // DEL /_config/:id remove it; the newest survivor is promoted if it was active // // GET /_officer/home views + resume + next-up + latest-per-view, one round trip diff --git a/src/servers/sidecar/transmission/index.ts b/src/servers/sidecar/transmission/index.ts index b3097521..96b36948 100644 --- a/src/servers/sidecar/transmission/index.ts +++ b/src/servers/sidecar/transmission/index.ts @@ -31,7 +31,7 @@ import { getTransmissionConfig } from './upstream'; // POST /_officer/torrents/rename {id, path, name} // POST /_officer/torrents/remove {ids, deleteLocalData} // GET /_officer/free-space?path= free space at a path -// POST /_officer/port-test is the peer port reachable from outside +// GET /_officer/port-test is the peer port reachable from outside // POST /_officer/blocklist-update refresh the blocklist, returns the new size // anything else 404 // diff --git a/src/servers/sidecar/transmission/routes.ts b/src/servers/sidecar/transmission/routes.ts index 461d3b4a..f4ceb0bc 100644 --- a/src/servers/sidecar/transmission/routes.ts +++ b/src/servers/sidecar/transmission/routes.ts @@ -198,8 +198,11 @@ async function handleFreeSpace(ctx: OfficerContext): Promise { return Response.json({ path: result.path, bytes: result['size-bytes'] }); } +// GET: it asks the daemon whether the peer port is reachable and returns the answer. Nothing changes, +// on this side or Transmission's. Blocklist-update below stays POST because it genuinely refetches and +// replaces the list — the two look alike and are not. async function handlePortTest(ctx: OfficerContext): Promise { - if (ctx.req.method !== 'POST') return methodNotAllowed(); + if (ctx.req.method !== 'GET') return methodNotAllowed(); const result = await rpc<{ 'port-is-open': boolean }>(ctx.userId, 'port-test'); return Response.json({ open: result['port-is-open'] }); } diff --git a/src/workspaces/officerdev/src/apps/Jellyfin/useJellyfinData.ts b/src/workspaces/officerdev/src/apps/Jellyfin/useJellyfinData.ts index ba04a2ec..1211cbea 100644 --- a/src/workspaces/officerdev/src/apps/Jellyfin/useJellyfinData.ts +++ b/src/workspaces/officerdev/src/apps/Jellyfin/useJellyfinData.ts @@ -282,7 +282,7 @@ export type AddServerInput = { label: string; url: string; username: string; pas export type EditServerInput = { id: number; label?: string; url?: string; username?: string; password?: string }; export function useJellyfinServerActions() { - const { post, patch, delete: del } = useClient(); + const { get, post, patch, delete: del } = useClient(); const qc = useQueryClient(); // Adding, editing, switching and removing all change what every other query here can even answer — a switch // in particular changes the answer to all of them without changing any of their inputs. @@ -309,11 +309,12 @@ export function useJellyfinServerActions() { onSuccess: invalidate, }); + // GET — probes one server and reports whether it answered. It does not switch to it (that is + // `activate`) and writes nothing. useMutation is still right: it is fired by a button, not rendered. const test = useMutation({ mutationFn: (id: number) => - post<{ ok: boolean; version?: string | null; serverName?: string | null; ms: number }>( + get<{ ok: boolean; version?: string | null; serverName?: string | null; ms: number }>( `/jellyfin/_config/${id}/test`, - {}, ), onSuccess: invalidate, }); diff --git a/src/workspaces/officerdev/src/apps/Transmission/useTransmissionData.ts b/src/workspaces/officerdev/src/apps/Transmission/useTransmissionData.ts index 95775ab3..709db813 100644 --- a/src/workspaces/officerdev/src/apps/Transmission/useTransmissionData.ts +++ b/src/workspaces/officerdev/src/apps/Transmission/useTransmissionData.ts @@ -197,11 +197,14 @@ export function useTorrentMutations() { } export function useMaintenance() { - const { post } = useClient(); + const { get, post } = useClient(); const qc = useQueryClient(); const portTest = useMutation({ - mutationFn: () => post<{ open: boolean }>('/transmission/_officer/port-test'), + // GET — it asks the daemon a question and changes nothing on either side. Still a useMutation + // because it is fired by a button rather than rendered from cache: that is a UI concern, not an + // HTTP one. blocklistUpdate below stays POST, because it really does refetch and replace the list. + mutationFn: () => get<{ open: boolean }>('/transmission/_officer/port-test'), onSuccess: (data) => (data.open ? toast.success('Peer port is open') : toast.error('Peer port is closed')), onError: (err) => toast.error(errorMessage(err, 'Port test failed')), }); diff --git a/src/workspaces/officerdev/src/hooks/useFilesAPI.ts b/src/workspaces/officerdev/src/hooks/useFilesAPI.ts index 915d0dcc..a5f9ea03 100644 --- a/src/workspaces/officerdev/src/hooks/useFilesAPI.ts +++ b/src/workspaces/officerdev/src/hooks/useFilesAPI.ts @@ -55,11 +55,14 @@ export const useFilesAPI = (root: string = 'home') => { client.get(withRoot(`/file-browser/download-video/${jobId}`)), // Prefetch one video's metadata (ReClip /api/info via the platform proxy). Returns { error } inline. - videoInfo: (url: string) => client.post(withRoot('/file-browser/video-info'), { url }), + videoInfo: (url: string) => + client.get(withRoot(`/file-browser/video-info?url=${encodeURIComponent(url)}`)), // Expand a playlist URL into its individual video URLs (ReClip /api/playlist). videoPlaylist: (url: string) => - client.post<{ urls?: string[]; error?: string }>(withRoot('/file-browser/video-playlist'), { url }), + client.get<{ urls?: string[]; error?: string }>( + withRoot(`/file-browser/video-playlist?url=${encodeURIComponent(url)}`), + ), tts: (path: string, opts?: { saveNextTo?: boolean }) => client.post<{ audioPath: string; audioRoot: string }>('/file-browser/tts', { path, root, ...opts }),