From c7bd494f11e792508534c1c795e24694498395c4 Mon Sep 17 00:00:00 2001 From: Ayobami Adegoke Date: Thu, 10 Sep 2026 19:36:54 +0100 Subject: [PATCH] fix(jobindex-search): reject non-jobindex detail URLs instead of fetching them verbatim (#447) (#448) detail fetched any http(s) input verbatim with no host check and, when the path didn't match, silently used the whole input URL as the job id - a non-posting page came back as a well-formed fake posting with exit 0. buildUrl now requires a jobindex.dk host and a /jobannonce/ path, rebuilds the fetch URL from the extracted id, and exits 1 with the stderr-JSON BAD_ID contract otherwise; bare ids stay permissive slash-free tokens per the jobnet precedent. Eight cases in the new detail-input.test.ts; the five rejection/canonicalization cases fail against the verbatim unguarded extraction. --- .../cli/src/commands/detail.ts | 55 +++++++++++++------ .../cli/tests/detail-input.test.ts | 52 ++++++++++++++++++ CHANGELOG.md | 19 +++++++ 3 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 .agents/skills/jobindex-search/cli/tests/detail-input.test.ts diff --git a/.agents/skills/jobindex-search/cli/src/commands/detail.ts b/.agents/skills/jobindex-search/cli/src/commands/detail.ts index f2dab2d..5f8be67 100644 --- a/.agents/skills/jobindex-search/cli/src/commands/detail.ts +++ b/.agents/skills/jobindex-search/cli/src/commands/detail.ts @@ -60,23 +60,38 @@ function stripTags(html: string): string { } /** - * Extract job ID from URL or return as-is if already an ID + * Parse a detail invocation's into a canonical fetch target, or null. + * + * This is the gate between a stored (untrusted) URL and a network fetch, so it + * must never trust the raw string: the previous version fetched any http(s) + * URL verbatim and, when the path didn't match, used the whole input URL as + * the id - a non-posting page (a redirect target, a look-alike host, the + * homepage) came back as a well-formed fake posting with exit 0 (#447). A URL + * input now needs a jobindex.dk host (apex or subdomain) and a + * /jobannonce/ path, and the fetch URL is rebuilt from the extracted id - + * the canonical short form the bare-id path always used. A bare id stays a + * permissive scheme- and slash-free token (the jobnet precedent): the server + * 404s unknowns loudly, which is the honest failure. Exported for tests. */ -function extractIdFromUrl(url: string): string { - // Match IDs like h1647303, r13677312, etc. - const match = url.match(/\/jobannonce\/([a-zA-Z]\d+)/) - if (match) return match[1] - return url -} - -function buildUrl(idOrUrl: string): { url: string; id: string } { - if (idOrUrl.startsWith("http")) { - const id = extractIdFromUrl(idOrUrl) - return { url: idOrUrl, id } +export function buildUrl(idOrUrl: string): { url: string; id: string } | null { + const trimmed = idOrUrl.trim() + if (/^https?:\/\//i.test(trimmed)) { + let host: string + try { + host = new URL(trimmed).hostname.toLowerCase() + } catch { + return null + } + if (host !== "jobindex.dk" && !host.endsWith(".jobindex.dk")) return null + // Match IDs like h1647303, r13677312, etc. + const match = trimmed.match(/\/jobannonce\/([a-zA-Z]\d+)/) + if (!match) return null + return { url: `${BASE_URL}/jobannonce/${match[1]}`, id: match[1] } } - // It's a bare ID - const url = `${BASE_URL}/jobannonce/${idOrUrl}` - return { url, id: idOrUrl } + if (/^[a-zA-Z0-9_-]+$/.test(trimmed)) { + return { url: `${BASE_URL}/jobannonce/${trimmed}`, id: trimmed } + } + return null } const DANISH_MONTHS: Record = { @@ -262,7 +277,15 @@ export const detail = defineCommand({ process.exit(1) } - const { url, id } = buildUrl(idArg) + const parsed = buildUrl(idArg) + if (!parsed) { + writeError( + `Could not parse a jobindex job id or jobannonce URL from "${idArg}"`, + "BAD_ID", + ) + process.exit(1) + } + const { url, id } = parsed try { const html = await htmlFetch(url) diff --git a/.agents/skills/jobindex-search/cli/tests/detail-input.test.ts b/.agents/skills/jobindex-search/cli/tests/detail-input.test.ts new file mode 100644 index 0000000..f7effd4 --- /dev/null +++ b/.agents/skills/jobindex-search/cli/tests/detail-input.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, test } from "bun:test"; +import { buildUrl } from "../src/commands/detail"; + +// buildUrl is the gate between a stored (untrusted) URL and a network fetch. +// It must yield a canonical jobindex fetch target or null (-> BAD_ID) - never +// the raw input. The unguarded version fetched any http(s) URL verbatim and, +// when the path didn't match, used the whole input URL as the id, so a +// non-posting page came back as a well-formed fake posting with exit 0 (#447). + +describe("jobindex detail input parsing", () => { + test("canonical URL with title slug", () => { + expect(buildUrl("https://www.jobindex.dk/jobannonce/h1647303/senior-data-engineer")).toEqual({ + url: "https://www.jobindex.dk/jobannonce/h1647303", + id: "h1647303", + }); + }); + + test("trailing slash and query string variants", () => { + expect(buildUrl("https://www.jobindex.dk/jobannonce/r13677312/")?.id).toBe("r13677312"); + expect(buildUrl("https://www.jobindex.dk/jobannonce/h1647303?utm_source=x")?.id).toBe("h1647303"); + }); + + test("jobindex subdomains and bare apex are accepted", () => { + expect(buildUrl("https://it.jobindex.dk/jobannonce/h1647303")?.id).toBe("h1647303"); + expect(buildUrl("https://jobindex.dk/jobannonce/h1647303")?.id).toBe("h1647303"); + }); + + test("a bare id builds the canonical URL (server 404s unknowns loudly)", () => { + expect(buildUrl("h1647303")).toEqual({ + url: "https://www.jobindex.dk/jobannonce/h1647303", + id: "h1647303", + }); + }); + + test("an off-host URL is rejected, not fetched", () => { + expect(buildUrl("https://evil.example/jobannonce/h1647303")).toBeNull(); + }); + + test("look-alike and userinfo hosts are rejected", () => { + expect(buildUrl("https://jobindex.dk.evil.example/jobannonce/h1647303")).toBeNull(); + expect(buildUrl("https://www.jobindex.dk@evil.example/jobannonce/h1647303")).toBeNull(); + }); + + test("an own-host URL without a jobannonce id is rejected (the fake-posting repro)", () => { + expect(buildUrl("https://www.jobindex.dk/")).toBeNull(); + }); + + test("garbage bare input is rejected", () => { + expect(buildUrl("not a slug!")).toBeNull(); + expect(buildUrl("ftp://www.jobindex.dk/jobannonce/h1")).toBeNull(); + }); +}); diff --git a/CHANGELOG.md b/CHANGELOG.md index 789afcc..1ca78ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,25 @@ per-file diff commands. ### Fixed +- **`jobindex-search detail` no longer fetches arbitrary URLs or invents posting-shaped + output** (#447) - the command fetched any `http(s)` input verbatim (no host check) and, + when the path didn't match its one pattern, silently used the whole input URL as the job + id; the only net was "the fetched page has a title", so a non-posting page came back as + a well-formed fake posting with exit 0 (demonstrated with jobindex's own homepage: + `id` = the URL, `title` = the site's tagline, `description` = navigation chrome). Every + other portal CLI rejects unparseable detail input with `BAD_ID` and constructs its fetch + URL from the extracted id; jobindex was the one CLI trusting the raw string - and + `/scrape`/`/rank` agents feed it stored URLs, so a ghost or redirected URL (the #331 + class) yielded plausible garbage instead of an error. `buildUrl` now requires a + jobindex.dk host (apex or subdomain - look-alike and userinfo tricks rejected via real + URL parsing) plus a `/jobannonce/` path, rebuilds the fetch URL from the extracted + id (the canonical short form the bare-id path always used), and exits 1 with the + stderr-JSON `BAD_ID` contract otherwise; bare ids stay permissive scheme- and + slash-free tokens (the jobnet precedent - the server 404s unknowns loudly). Pinned by + eight cases in the new `detail-input.test.ts`; the five rejection/canonicalization + cases fail against the verbatim unguarded extraction. Complementary to the `/apply` + host-check rule proposed in #431, which stays with its proposer. + - **`/outcome` and `/interview` no longer confuse two roles at the same company** (#443) (`.claude/commands/outcome.md`, `.claude/commands/interview.md`, `tests/test_apply_records_application.py`) - when a tracker row's `cv_file` /