app store: never assume a service is local, or on its usual port
An instance may be on another host, behind a reverse proxy on 443 under a path prefix, on a tailnet address, or on an arbitrary port because the usual one was taken. All ordinary self-hosted setups, and each one a case where assuming otherwise produces a connection that fails later with no clue why. Two places were sloppy about it. Jellyfin's placeholder read `http://localhost:8096` and Transmission's `http://localhost:9091`, which quietly teach that a service must be local and on its project's default port; both now show remote examples, and the field type says why. And nothing validated what was typed, so a bare hostname or a URL with a token in the query string was stored as-is. The rule: reject only what cannot work, normalise what is merely untidy, have no opinion about the rest. No check that the host is local, that the port matches a default, or that the scheme is https — a tailnet HTTP service is completely normal. Trailing slashes go, because `${url}/api/x` otherwise doubles the separator: accepted by some servers and 404 by others, which is the kind of difference that reproduces on one machine and not another. Query strings go, because that is where a token hides, and it would sit in a column meant for a location. Credentials in the URL are refused for the same reason — outside the encrypted secret, and in every log line that ever prints it. A missing scheme is named rather than called invalid: it is the commonest mistake, because it is what people type into a browser. Verified through the API: `memos.example.com` is blocked with the fix quoted back, and `https://memos.example.com:8443/memos/` installs and stores normalised — remote host, non-standard port, path prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,66 @@
|
||||
import { describe, expect, it } from 'bun:test';
|
||||
import { checkServiceUrl } from './url';
|
||||
|
||||
// The instance may be anywhere. These tests are mostly about what must NOT be rejected — every case
|
||||
// below is an ordinary self-hosted setup, and treating any of them as invalid would send someone to
|
||||
// the docs for no reason.
|
||||
|
||||
describe('accepts wherever the service actually is', () => {
|
||||
const ok = [
|
||||
'https://photos.example.com', // remote, behind a proxy on 443
|
||||
'http://10.0.0.5:8096', // another host on the LAN, standard port
|
||||
'http://100.64.0.1:19999', // tailnet address, arbitrary port
|
||||
'https://example.com/immich', // sharing a domain, served under a path
|
||||
'http://127.0.0.1:9091', // same machine, which is allowed — just not assumed
|
||||
'http://immich:2283', // a docker network hostname
|
||||
];
|
||||
|
||||
for (const url of ok) {
|
||||
it(`accepts ${url}`, () => {
|
||||
expect(checkServiceUrl(url).ok).toBe(true);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('normalises what is untidy', () => {
|
||||
it('strips a trailing slash', () => {
|
||||
// `${url}/api/x` would otherwise double the separator — accepted by some servers, 404 by others,
|
||||
// which is the kind of difference that reproduces on one machine and not another.
|
||||
const result = checkServiceUrl('https://photos.example.com/');
|
||||
expect(result.ok && result.url).toBe('https://photos.example.com');
|
||||
});
|
||||
|
||||
it('drops a query string, which is where a token would hide', () => {
|
||||
const result = checkServiceUrl('https://x.example.com/?token=secret');
|
||||
expect(result.ok && result.url).toBe('https://x.example.com');
|
||||
});
|
||||
|
||||
it('keeps a path, because a service under a prefix is normal', () => {
|
||||
const result = checkServiceUrl('https://example.com/immich/');
|
||||
expect(result.ok && result.url).toBe('https://example.com/immich');
|
||||
});
|
||||
});
|
||||
|
||||
describe('rejects only what cannot work', () => {
|
||||
it('names the missing scheme rather than saying "invalid"', () => {
|
||||
// The commonest mistake: people type what they type into a browser.
|
||||
const result = checkServiceUrl('photos.example.com');
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error('unreachable');
|
||||
expect(result.error).toContain('https://photos.example.com');
|
||||
});
|
||||
|
||||
it('refuses credentials in the URL', () => {
|
||||
// They would land in a column meant for a location — outside the encrypted secret, and in every
|
||||
// log line that ever prints the URL.
|
||||
expect(checkServiceUrl('https://user:pass@x.example.com').ok).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses a scheme nothing here speaks', () => {
|
||||
expect(checkServiceUrl('ftp://x.example.com').ok).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses empty', () => {
|
||||
expect(checkServiceUrl(' ').ok).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user