api: five reads that were declared as writes are now GET
The permission model being built reads the HTTP method to decide whether a
non-owner may make a call: safe methods are reads, everything else is a write.
That only works if the method tells the truth. These five read something and
returned it while announcing themselves as writes, so a member would have been
denied a read they are entitled to because of a habit in how the route was
declared.
/api/file-browser/video-info POST {url} -> GET ?url=
/api/file-browser/video-playlist POST {url} -> GET ?url=
/api/server-settings/ocr/models POST {url} -> GET ?url=
/api/transmission/_officer/port-test POST -> GET
/api/jellyfin/_config/:id/test POST|GET -> GET only
The last one already answered to both, which is worse than either: a method that
means nothing cannot be the thing authorisation reads.
Deliberately stops at five. A sweep of all 100 mutating routes found many more
reads wearing POST, and they are staying, for two reasons that are not going
away: some need a request body GET cannot carry (/stt takes multipart audio;
/tts, /ocr, /transcribe take payloads), and some carry a credential, where a
query string is the wrong place — access logs, shell history and Referer headers
all capture those, request bodies do not (/tts/voices takes an apiKey, the four
/test endpoints take connection secrets, /local-providers/probe takes auth).
So the method alone can never carry the permission model, and the registry will
need an explicit per-route classification regardless. Converting these five is
worth it because it is free; converting the rest would be a breaking change
across 117 mobile call sites that buys nothing.
Web callers updated in the same commit; the sidecar contract comments now match.
Mobile has exactly one caller to change — transmissionPortTest in
packages/core/src/services/transmission.ts — and no shim was added, because an
endpoint answering to both methods is the problem this commit exists to fix.
docs/api-method-changes-2026-08-06.md is the handoff for the mobile team: what
changed, the one line to edit, what deliberately did NOT change and why, and how
to verify.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||||
@@ -32,7 +32,9 @@ export const OCRSection = () => {
|
|||||||
const fetchModels = async (u: string) => {
|
const fetchModels = async (u: string) => {
|
||||||
setModelsLoading(true);
|
setModelsLoading(true);
|
||||||
try {
|
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);
|
if (res.models) setModels(res.models);
|
||||||
else setModels([]);
|
else setModels([]);
|
||||||
} catch {
|
} 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"
|
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"
|
title="Refresh models"
|
||||||
>
|
>
|
||||||
<RefreshCw className={`h-3 w-3 text-duck-dark/50 dark:text-foreground/50 ${modelsLoading ? 'animate-spin' : ''}`} />
|
<RefreshCw
|
||||||
|
className={`h-3 w-3 text-duck-dark/50 dark:text-foreground/50 ${modelsLoading ? 'animate-spin' : ''}`}
|
||||||
|
/>
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
{models.length > 0 ? (
|
{models.length > 0 ? (
|
||||||
@@ -121,7 +125,9 @@ export const OCRSection = () => {
|
|||||||
</SelectTrigger>
|
</SelectTrigger>
|
||||||
<SelectContent className="z-[600] max-h-[300px]">
|
<SelectContent className="z-[600] max-h-[300px]">
|
||||||
{models.map((m) => (
|
{models.map((m) => (
|
||||||
<SelectItem key={m} value={m}>{m}</SelectItem>
|
<SelectItem key={m} value={m}>
|
||||||
|
{m}
|
||||||
|
</SelectItem>
|
||||||
))}
|
))}
|
||||||
</SelectContent>
|
</SelectContent>
|
||||||
</Select>
|
</Select>
|
||||||
|
|||||||
@@ -1331,8 +1331,10 @@ router.get('/download-video/:jobId', (ctx) => {
|
|||||||
// Prefetch a single video's metadata (title / thumbnail / duration / uploader) — proxied straight to
|
// 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
|
// 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.
|
// video, timeout, …) come back as { error } with a 200 so the client can render them inline.
|
||||||
router.post('/video-info', async (ctx) => {
|
// GET, not POST: it reads and writes nothing. The method is not cosmetic — the permission model reads
|
||||||
const { url } = ctx.get('body') as { url?: string };
|
// 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');
|
if (!url) throw errors.BAD_REQUEST('url is required');
|
||||||
const res = await fetch(`${RECLIP_BASE}/api/info`, {
|
const res = await fetch(`${RECLIP_BASE}/api/info`, {
|
||||||
method: 'POST',
|
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
|
// 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.
|
// then prefetches /video-info per url to build the per-entry cards.
|
||||||
router.post('/video-playlist', async (ctx) => {
|
// GET for the same reason as /video-info above.
|
||||||
const { url } = ctx.get('body') as { url?: string };
|
router.get('/video-playlist', async (ctx) => {
|
||||||
|
const url = ctx.req.query('url');
|
||||||
if (!url) throw errors.BAD_REQUEST('url is required');
|
if (!url) throw errors.BAD_REQUEST('url is required');
|
||||||
const res = await fetch(`${RECLIP_BASE}/api/playlist`, {
|
const res = await fetch(`${RECLIP_BASE}/api/playlist`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
|
|||||||
@@ -27,8 +27,11 @@ ocrRouter.put('/', async (ctx) => {
|
|||||||
return ctx.json({ success: true });
|
return ctx.json({ success: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
ocrRouter.post('/models', async (ctx) => {
|
// GET: it asks a provider what models it has and returns the answer. Nothing is written, and the only
|
||||||
const body = await ctx.req.json<{ url: string }>();
|
// 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);
|
if (!body.url) return ctx.json({ error: 'URL required' }, 400);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -226,8 +226,11 @@ export async function handleConfigRoute(req: Request, userId: number, subpath: s
|
|||||||
return serverList(userId);
|
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 (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);
|
return testServer(userId, id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ import { getConfig, probe } from './upstream';
|
|||||||
// for an access token and never stored
|
// for an access token and never stored
|
||||||
// PATCH /_config/:id same fields, all optional; no password keeps the stored token
|
// PATCH /_config/:id same fields, all optional; no password keeps the stored token
|
||||||
// POST /_config/:id/activate switch to that server
|
// 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
|
// 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
|
// GET /_officer/home views + resume + next-up + latest-per-view, one round trip
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ import { getTransmissionConfig } from './upstream';
|
|||||||
// POST /_officer/torrents/rename {id, path, name}
|
// POST /_officer/torrents/rename {id, path, name}
|
||||||
// POST /_officer/torrents/remove {ids, deleteLocalData}
|
// POST /_officer/torrents/remove {ids, deleteLocalData}
|
||||||
// GET /_officer/free-space?path= free space at a path
|
// 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
|
// POST /_officer/blocklist-update refresh the blocklist, returns the new size
|
||||||
// anything else 404
|
// anything else 404
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -198,8 +198,11 @@ async function handleFreeSpace(ctx: OfficerContext): Promise<Response> {
|
|||||||
return Response.json({ path: result.path, bytes: result['size-bytes'] });
|
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<Response> {
|
async function handlePortTest(ctx: OfficerContext): Promise<Response> {
|
||||||
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');
|
const result = await rpc<{ 'port-is-open': boolean }>(ctx.userId, 'port-test');
|
||||||
return Response.json({ open: result['port-is-open'] });
|
return Response.json({ open: result['port-is-open'] });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 type EditServerInput = { id: number; label?: string; url?: string; username?: string; password?: string };
|
||||||
|
|
||||||
export function useJellyfinServerActions() {
|
export function useJellyfinServerActions() {
|
||||||
const { post, patch, delete: del } = useClient();
|
const { get, post, patch, delete: del } = useClient();
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
// Adding, editing, switching and removing all change what every other query here can even answer — a switch
|
// 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.
|
// in particular changes the answer to all of them without changing any of their inputs.
|
||||||
@@ -309,11 +309,12 @@ export function useJellyfinServerActions() {
|
|||||||
onSuccess: invalidate,
|
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({
|
const test = useMutation({
|
||||||
mutationFn: (id: number) =>
|
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`,
|
`/jellyfin/_config/${id}/test`,
|
||||||
{},
|
|
||||||
),
|
),
|
||||||
onSuccess: invalidate,
|
onSuccess: invalidate,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -197,11 +197,14 @@ export function useTorrentMutations() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function useMaintenance() {
|
export function useMaintenance() {
|
||||||
const { post } = useClient();
|
const { get, post } = useClient();
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
|
|
||||||
const portTest = useMutation({
|
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')),
|
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')),
|
onError: (err) => toast.error(errorMessage(err, 'Port test failed')),
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -55,11 +55,14 @@ export const useFilesAPI = (root: string = 'home') => {
|
|||||||
client.get<DownloadVideoStatus>(withRoot(`/file-browser/download-video/${jobId}`)),
|
client.get<DownloadVideoStatus>(withRoot(`/file-browser/download-video/${jobId}`)),
|
||||||
|
|
||||||
// Prefetch one video's metadata (ReClip /api/info via the platform proxy). Returns { error } inline.
|
// Prefetch one video's metadata (ReClip /api/info via the platform proxy). Returns { error } inline.
|
||||||
videoInfo: (url: string) => client.post<VideoInfo>(withRoot('/file-browser/video-info'), { url }),
|
videoInfo: (url: string) =>
|
||||||
|
client.get<VideoInfo>(withRoot(`/file-browser/video-info?url=${encodeURIComponent(url)}`)),
|
||||||
|
|
||||||
// Expand a playlist URL into its individual video URLs (ReClip /api/playlist).
|
// Expand a playlist URL into its individual video URLs (ReClip /api/playlist).
|
||||||
videoPlaylist: (url: string) =>
|
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 }) =>
|
tts: (path: string, opts?: { saveNextTo?: boolean }) =>
|
||||||
client.post<{ audioPath: string; audioRoot: string }>('/file-browser/tts', { path, root, ...opts }),
|
client.post<{ audioPath: string; audioRoot: string }>('/file-browser/tts', { path, root, ...opts }),
|
||||||
|
|||||||
Reference in New Issue
Block a user