mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix: request registry tokens with the target repository scope (#1478)
* fix: request registry tokens with the target repository scope The image-update detector authenticated to registries by reusing the scope echoed in the registry's GET /v2/ ping. That ping carries no repository context, and ghcr.io answers it with a placeholder scope (repository:user/image:pull), so the token was requested for the wrong repository and rejected. Every ghcr.io-backed image (including lscr.io, which delegates auth to ghcr.io) then failed its manifest lookup and was reported as "Registry unreachable", while Docker Hub and quay.io kept working. Always request a pull scope for the repository being checked rather than the echoed placeholder. Also report the actual failure cause: getRemoteDigestResult now distinguishes an authentication failure, a rate limit (with retry-after), a missing image, a registry error, and a genuinely unreachable registry, instead of collapsing every failure into "Registry unreachable". getRemoteDigest stays a digest-or-null wrapper so the update-preview path is unchanged, and listRegistryTags shares the same token path so it now resolves on ghcr.io/lscr.io too. * fix: neutralize control characters in the registry digest error log The error-path console.error in getRemoteDigestResult interpolated the image ref and the caught error message, both of which originate from compose-authored input. Route them through sanitizeForLog so a crafted image string or upstream error text cannot forge multi-line log entries (log injection). The returned reason and the digest logic are unchanged.
This commit is contained in:
Anso
@@ -94,6 +94,14 @@ vi.mock('../services/NodeRegistry', () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
// getRemoteDigestResult is module-scoped inside checkImage; mock it to drive the remote
|
||||
// outcome while keeping the real parseImageRef / repoDigestMatchesRef.
|
||||
const { mockGetRemoteDigestResult } = vi.hoisted(() => ({ mockGetRemoteDigestResult: vi.fn() }));
|
||||
vi.mock('../services/registry-api', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../services/registry-api')>();
|
||||
return { ...actual, getRemoteDigestResult: mockGetRemoteDigestResult };
|
||||
});
|
||||
|
||||
// ── Re-export internal helpers via the module ─────────────────────────
|
||||
|
||||
// We need the internal functions. Import the module after mocks are set up.
|
||||
@@ -186,6 +194,38 @@ describe('ImageUpdateService - image ref parsing (via checkImage)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── checkImage surfaces the remote-digest reason ───────────────────────
|
||||
|
||||
describe('ImageUpdateService - checkImage surfaces the remote-digest reason', () => {
|
||||
let service: ImageUpdateService;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
(ImageUpdateService as any).instance = undefined;
|
||||
service = ImageUpdateService.getInstance();
|
||||
});
|
||||
|
||||
// One RepoDigest matching the ref so the local digest resolves and the flow reaches
|
||||
// getRemoteDigestResult.
|
||||
const dockerWithLocalDigest = (digest: string) => ({
|
||||
getDocker: () => ({
|
||||
getImage: () => ({ inspect: vi.fn().mockResolvedValue({ RepoDigests: [`ghcr.io/linuxserver/radarr@${digest}`] }) }),
|
||||
}),
|
||||
} as any);
|
||||
|
||||
it('surfaces the specific failure reason (not a generic "unreachable") as the check error', async () => {
|
||||
mockGetRemoteDigestResult.mockResolvedValue({ ok: false, reason: 'Authentication failed for ghcr.io/linuxserver/radarr:latest' });
|
||||
const result = await service.checkImage(dockerWithLocalDigest('sha256:local'), 'ghcr.io/linuxserver/radarr:latest');
|
||||
expect(result).toEqual({ hasUpdate: false, error: 'Authentication failed for ghcr.io/linuxserver/radarr:latest' });
|
||||
});
|
||||
|
||||
it('reports an update when the resolved remote digest differs from the local one', async () => {
|
||||
mockGetRemoteDigestResult.mockResolvedValue({ ok: true, digest: 'sha256:remote' });
|
||||
const result = await service.checkImage(dockerWithLocalDigest('sha256:local'), 'ghcr.io/linuxserver/radarr:latest');
|
||||
expect(result).toEqual({ hasUpdate: true });
|
||||
});
|
||||
});
|
||||
|
||||
// ── Rate limiting ─────────────────────────────────────────────────────
|
||||
|
||||
describe('ImageUpdateService - manual refresh cooldown', () => {
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
/**
|
||||
* Unit tests for the registry HTTP client: digest-name matching (the local
|
||||
* RepoDigest vs image-ref comparison) and getRemoteDigest's HEAD-first lookup
|
||||
* with GET fallback.
|
||||
* RepoDigest vs image-ref comparison), getAuthToken's token-scope construction,
|
||||
* getRemoteDigest's HEAD-first lookup with GET fallback, and getRemoteDigestResult's
|
||||
* status-to-reason mapping.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { EventEmitter } from 'events';
|
||||
@@ -38,7 +39,7 @@ function fakeRequest(url: string, options: { method?: string }, cb: (res: EventE
|
||||
vi.mock('https', () => ({ default: { request: (...args: unknown[]) => fakeRequest(...(args as Parameters<typeof fakeRequest>)) } }));
|
||||
vi.mock('http', () => ({ default: { request: (...args: unknown[]) => fakeRequest(...(args as Parameters<typeof fakeRequest>)) } }));
|
||||
|
||||
import { repoDigestMatchesRef, getRemoteDigest, parseImageRef } from '../services/registry-api';
|
||||
import { repoDigestMatchesRef, getRemoteDigest, getRemoteDigestResult, getAuthToken, parseImageRef } from '../services/registry-api';
|
||||
|
||||
const TOKEN_BODY = JSON.stringify({ token: 'test-token' });
|
||||
const REMOTE = 'sha256:remote000000000000000000000000000000000000000000000000000000';
|
||||
@@ -132,3 +133,105 @@ describe('getRemoteDigest HEAD-first lookup', () => {
|
||||
expect(calls.filter(c => c.url.includes('/manifests/')).map(c => c.method)).toEqual(['HEAD']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAuthToken builds the token request for the target repository', () => {
|
||||
beforeEach(() => {
|
||||
calls.length = 0;
|
||||
});
|
||||
|
||||
// ghcr.io (and lscr.io, which delegates auth to it) echo a placeholder scope in the
|
||||
// context-less /v2/ ping. The token must be requested for the repo we actually want;
|
||||
// reusing the echoed scope made ghcr.io mint a token for the wrong repo and reject it.
|
||||
const GHCR_CHALLENGE = 'Bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:user/image:pull"';
|
||||
|
||||
it('ignores the placeholder scope echoed by the /v2/ ping and uses the target repo', async () => {
|
||||
route = (url, method): FakeResp => {
|
||||
if (url === 'https://ghcr.io/v2/') return { statusCode: 401, headers: { 'www-authenticate': GHCR_CHALLENGE } };
|
||||
if (url.startsWith('https://ghcr.io/token')) return { statusCode: 200, headers: {}, body: TOKEN_BODY };
|
||||
return method === 'HEAD' ? { statusCode: 200, headers: { 'docker-content-digest': REMOTE } } : { statusCode: 500, headers: {} };
|
||||
};
|
||||
const token = await getAuthToken('ghcr.io', 'linuxserver/radarr', null);
|
||||
expect(token).toBe('test-token');
|
||||
const tokenCall = calls.find(c => c.url.startsWith('https://ghcr.io/token'));
|
||||
expect(tokenCall).toBeTruthy();
|
||||
const decoded = decodeURIComponent(tokenCall?.url ?? '');
|
||||
expect(decoded).toContain('scope=repository:linuxserver/radarr:pull');
|
||||
expect(decoded).not.toContain('user/image');
|
||||
});
|
||||
|
||||
it('returns null when the token endpoint rejects the request (403)', async () => {
|
||||
route = (url): FakeResp => {
|
||||
if (url === 'https://ghcr.io/v2/') return { statusCode: 401, headers: { 'www-authenticate': GHCR_CHALLENGE } };
|
||||
if (url.startsWith('https://ghcr.io/token')) return { statusCode: 403, headers: {} };
|
||||
return { statusCode: 200, headers: { 'docker-content-digest': REMOTE } };
|
||||
};
|
||||
expect(await getAuthToken('ghcr.io', 'linuxserver/radarr', null)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getRemoteDigestResult failure reasons', () => {
|
||||
beforeEach(() => {
|
||||
calls.length = 0;
|
||||
});
|
||||
|
||||
const REF = 'registry-1.docker.io/library/nginx:latest';
|
||||
const get = () => getRemoteDigestResult('registry-1.docker.io', 'library/nginx', 'latest');
|
||||
// Token always succeeds; the HEAD response under test drives the outcome.
|
||||
const headResp = (resp: FakeResp) => (url: string, method: string): FakeResp =>
|
||||
tokenOk(url) ?? (method === 'HEAD' ? resp : { statusCode: 200, headers: { 'docker-content-digest': REMOTE } });
|
||||
|
||||
it('returns the digest on a HEAD 200', async () => {
|
||||
route = headResp({ statusCode: 200, headers: { 'docker-content-digest': REMOTE } });
|
||||
expect(await get()).toEqual({ ok: true, digest: REMOTE });
|
||||
});
|
||||
|
||||
it('maps 401 to an authentication failure', async () => {
|
||||
route = headResp({ statusCode: 401, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Authentication failed for ${REF}` });
|
||||
});
|
||||
|
||||
it('maps 429 to a rate-limit reason including retry-after', async () => {
|
||||
route = headResp({ statusCode: 429, headers: { 'retry-after': '3600' } });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Rate limited by registry for ${REF} (retry after 3600)` });
|
||||
});
|
||||
|
||||
it('maps 429 without retry-after to a plain rate-limit reason', async () => {
|
||||
route = headResp({ statusCode: 429, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Rate limited by registry for ${REF}` });
|
||||
});
|
||||
|
||||
it('maps 403 to an authentication failure', async () => {
|
||||
route = headResp({ statusCode: 403, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Authentication failed for ${REF}` });
|
||||
});
|
||||
|
||||
it('maps an unexpected status to a generic reason with the status code', async () => {
|
||||
route = headResp({ statusCode: 400, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Registry returned status 400 for ${REF}` });
|
||||
});
|
||||
|
||||
it('maps 404 to image not found', async () => {
|
||||
route = headResp({ statusCode: 404, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Image not found: ${REF}` });
|
||||
});
|
||||
|
||||
it('maps 5xx to a registry error with the status', async () => {
|
||||
route = headResp({ statusCode: 503, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Registry error (503) for ${REF}` });
|
||||
});
|
||||
|
||||
it('derives the reason from the GET fallback when HEAD is 405', async () => {
|
||||
route = (url, method) => tokenOk(url) ?? (method === 'HEAD' ? { statusCode: 405, headers: {} } : { statusCode: 401, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Authentication failed for ${REF}` });
|
||||
});
|
||||
|
||||
it('fails when both HEAD and GET are 200 but omit the digest header', async () => {
|
||||
route = (url, method) => tokenOk(url) ?? (method === 'HEAD' ? { statusCode: 200, headers: {} } : { statusCode: 200, headers: {} });
|
||||
expect(await get()).toEqual({ ok: false, reason: `Registry returned no digest for ${REF}` });
|
||||
});
|
||||
|
||||
it('reports unreachable when the request throws, including the error cause', async () => {
|
||||
route = () => { throw new Error('ENOTFOUND'); };
|
||||
expect(await get()).toEqual({ ok: false, reason: `Registry unreachable for ${REF} (ENOTFOUND)` });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,7 +8,7 @@ import { RegistryService } from './RegistryService';
|
||||
import { NodeRegistry } from './NodeRegistry';
|
||||
import { NotificationService } from './NotificationService';
|
||||
import { sanitizeNotificationMessage } from '../utils/notificationMessage';
|
||||
import { parseImageRef, getRemoteDigest, repoDigestMatchesRef } from './registry-api';
|
||||
import { parseImageRef, getRemoteDigestResult, repoDigestMatchesRef } from './registry-api';
|
||||
import { isDebugEnabled } from '../utils/debug';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
@@ -705,10 +705,11 @@ export class ImageUpdateService {
|
||||
return { hasUpdate: false, error: `Could not resolve a local registry digest for "${imageRef}"` };
|
||||
}
|
||||
|
||||
const remoteDigest = await getRemoteDigest(parsed.registry, parsed.repo, parsed.tag, credentials);
|
||||
if (!remoteDigest) {
|
||||
return { hasUpdate: false, error: `Registry unreachable for ${parsed.registry}/${parsed.repo}:${parsed.tag}` };
|
||||
const remote = await getRemoteDigestResult(parsed.registry, parsed.repo, parsed.tag, credentials);
|
||||
if (!remote.ok) {
|
||||
return { hasUpdate: false, error: remote.reason };
|
||||
}
|
||||
const remoteDigest = remote.digest;
|
||||
|
||||
const hasUpdate = localDigest !== remoteDigest;
|
||||
console.log(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import https from 'https';
|
||||
import http from 'http';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
|
||||
export interface ParsedRef {
|
||||
registry: string;
|
||||
@@ -113,12 +114,15 @@ export async function getAuthToken(
|
||||
|
||||
const realmMatch = wwwAuth.match(/realm="([^"]+)"/);
|
||||
const serviceMatch = wwwAuth.match(/service="([^"]+)"/);
|
||||
const scopeMatch = wwwAuth.match(/scope="([^"]+)"/);
|
||||
if (!realmMatch) return null;
|
||||
|
||||
const params = new URLSearchParams();
|
||||
if (serviceMatch) params.set('service', serviceMatch[1]);
|
||||
params.set('scope', scopeMatch ? scopeMatch[1] : `repository:${repo}:pull`);
|
||||
// The /v2/ ping carries no repository context, so any scope it echoes is a
|
||||
// placeholder (ghcr.io returns repository:user/image:pull). Always request
|
||||
// the scope for the repository we actually want; reusing the echoed scope
|
||||
// makes ghcr.io mint a token for the wrong repo and then reject the pull.
|
||||
params.set('scope', `repository:${repo}:pull`);
|
||||
tokenUrl = `${realmMatch[1]}?${params.toString()}`;
|
||||
}
|
||||
|
||||
@@ -164,43 +168,98 @@ export function repoDigestMatchesRef(repoDigest: string, parsed: ParsedRef): boo
|
||||
&& parsedName.repo === parsed.repo;
|
||||
}
|
||||
|
||||
export async function getRemoteDigest(
|
||||
/** Outcome of a remote-digest lookup: the digest, or a human-readable reason it failed. */
|
||||
export type RemoteDigestResult =
|
||||
| { ok: true; digest: string }
|
||||
| { ok: false; reason: string };
|
||||
|
||||
/**
|
||||
* Map a non-success manifest status to a specific reason so a caller can tell an auth
|
||||
* failure from a rate limit, a missing image, or a server error, rather than collapsing
|
||||
* them all into "unreachable". `ref` is the resolved "<registry>/<repo>:<tag>" for the
|
||||
* image after Docker Hub normalization, not necessarily the literal string the user wrote.
|
||||
*/
|
||||
function manifestFailureReason(statusCode: number, ref: string, headers: HttpResult['headers']): string {
|
||||
if (statusCode === 401 || statusCode === 403) return `Authentication failed for ${ref}`;
|
||||
if (statusCode === 429) {
|
||||
const retry = headers['retry-after'];
|
||||
const retryStr = Array.isArray(retry) ? retry[0] : retry;
|
||||
return retryStr
|
||||
? `Rate limited by registry for ${ref} (retry after ${retryStr})`
|
||||
: `Rate limited by registry for ${ref}`;
|
||||
}
|
||||
if (statusCode === 404) return `Image not found: ${ref}`;
|
||||
if (statusCode >= 500) return `Registry error (${statusCode}) for ${ref}`;
|
||||
return `Registry returned status ${statusCode} for ${ref}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the remote manifest digest for an image, returning either the digest or the
|
||||
* reason the lookup failed. Same HEAD-first/GET-fallback transport as before (HEAD
|
||||
* returns docker-content-digest without transferring the body, so it does not draw down
|
||||
* Docker Hub's anonymous pull-rate budget the way a GET can); only the failure handling
|
||||
* is richer. A 401/403/404/429/5xx HEAD reports its specific reason without a GET retry,
|
||||
* since the bearer token is fetched up-front, so a 401 here is a real auth failure rather
|
||||
* than a token-scope challenge to retry.
|
||||
*/
|
||||
export async function getRemoteDigestResult(
|
||||
registry: string,
|
||||
repo: string,
|
||||
tag: string,
|
||||
credentials?: RegistryCredentials | null,
|
||||
): Promise<string | null> {
|
||||
): Promise<RemoteDigestResult> {
|
||||
const ref = `${registry}/${repo}:${tag}`;
|
||||
try {
|
||||
const token = await getAuthToken(registry, repo, credentials);
|
||||
const headers: Record<string, string> = { Accept: MANIFEST_ACCEPT };
|
||||
if (token) headers['Authorization'] = `Bearer ${token}`;
|
||||
const url = `https://${registry}/v2/${repo}/manifests/${tag}`;
|
||||
|
||||
// HEAD first: the registry returns docker-content-digest without
|
||||
// transferring the manifest body, so it does not draw down Docker Hub's
|
||||
// anonymous pull-rate budget the way a GET does (a GET can self-inflict a
|
||||
// 429). Fall back to GET only when the registry rejects HEAD (405/501) or
|
||||
// omits the digest header on a 200. A 401/403/404/429/5xx HEAD returns
|
||||
// null without a GET retry: the bearer token is fetched up-front, so a
|
||||
// 401 here is a real auth failure, not a token-scope challenge to retry.
|
||||
const head = await httpRequest(url, 'HEAD', headers);
|
||||
if (head.statusCode === 200) {
|
||||
const digest = head.headers['docker-content-digest'];
|
||||
if (typeof digest === 'string') return digest;
|
||||
if (typeof digest === 'string') return { ok: true, digest };
|
||||
// 200 without the digest header: fall through to GET to read it from there.
|
||||
} else if (head.statusCode !== 405 && head.statusCode !== 501) {
|
||||
// 401/403/404/429/5xx: a GET would fail the same way. Report as unreachable.
|
||||
return null;
|
||||
return { ok: false, reason: manifestFailureReason(head.statusCode, ref, head.headers) };
|
||||
}
|
||||
|
||||
const res = await httpRequest(url, 'GET', headers);
|
||||
if (res.statusCode !== 200) return null;
|
||||
const digest = res.headers['docker-content-digest'];
|
||||
return typeof digest === 'string' ? digest : null;
|
||||
} catch {
|
||||
return null;
|
||||
if (res.statusCode === 200) {
|
||||
const digest = res.headers['docker-content-digest'];
|
||||
if (typeof digest === 'string') return { ok: true, digest };
|
||||
// 200 on both HEAD and GET but no digest header: a spec-violating registry.
|
||||
return { ok: false, reason: `Registry returned no digest for ${ref}` };
|
||||
}
|
||||
return { ok: false, reason: manifestFailureReason(res.statusCode, ref, res.headers) };
|
||||
} catch (e) {
|
||||
// Bind and log the cause: a bare catch here would flatten DNS, TLS, connection-
|
||||
// refused, and timeout failures into one opaque string with nothing in the logs,
|
||||
// the silent-failure mode this function exists to remove. Prefer the errno code
|
||||
// (ENOTFOUND/ECONNREFUSED/ETIMEDOUT/...) over a verbose message so the reason
|
||||
// stays short in the sidebar tooltip; fall back to the message otherwise.
|
||||
const cause = e instanceof Error ? ((e as NodeJS.ErrnoException).code ?? e.message) : String(e);
|
||||
// ref and cause derive from the compose-authored image string and upstream error
|
||||
// text, so neutralize control characters before they reach the log line.
|
||||
console.error(`[registry-api] Remote digest lookup for ${sanitizeForLog(ref)} failed:`, sanitizeForLog(cause));
|
||||
return { ok: false, reason: `Registry unreachable for ${ref} (${cause})` };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Digest-or-null view of {@link getRemoteDigestResult} for callers that only need the
|
||||
* digest and treat any failure as "unknown" (e.g. the update-preview tag/digest diff).
|
||||
*/
|
||||
export async function getRemoteDigest(
|
||||
registry: string,
|
||||
repo: string,
|
||||
tag: string,
|
||||
credentials?: RegistryCredentials | null,
|
||||
): Promise<string | null> {
|
||||
const result = await getRemoteDigestResult(registry, repo, tag, credentials);
|
||||
return result.ok ? result.digest : null;
|
||||
}
|
||||
|
||||
export async function listRegistryTags(
|
||||
registry: string,
|
||||
repo: string,
|
||||
|
||||
Reference in New Issue
Block a user