conductor

CI task system
git clone git://git.finwo.net/app/conductor
Log | Files | Refs | README | LICENSE

commit b0e0b12cd0c70ae9bdb5cf018e034eaee590c553
parent 37b65dbede0a4dc12684f4f95d0b3eb42ae76500
Author: finwo <finwo@pm.me>
Date:   Sun, 27 Sep 2026 00:14:07 +0200

Require proof of authority to start a build, read one back and set visibility

Diffstat:
M.conductor.yml | 1-
MREADME.md | 15+++++++++------
Mdeploy/smoke.sh | 3+--
Mdocs/api.md | 21+++++++++++++--------
Mdocs/auth.md | 9++++++---
Mdocs/pipeline.md | 17++++++-----------
Msrc/admin-cli.js | 3++-
Msrc/conductor/routes/api.js | 13+++++++++++++
Msrc/conductor/routes/jobs.js | 69+++++++++++++++++++--------------------------------------------------
Msrc/conductor/routes/projects.js | 2+-
Msrc/conductor/routes/tasks.js | 41++++++++---------------------------------
Msrc/conductor/routes/users.js | 7+++++++
Msrc/conductor/scheduler.js | 2+-
Msrc/lib/git.js | 4++++
Msrc/lib/pipeline/index.js | 3+--
Msrc/lib/pipeline/parse.js | 26++++++++++----------------
Msrc/lib/projects.js | 36+++++++++++++++++++++++++++++++++---
Mtest/api.test.js | 4++--
Mtest/conductor.test.js | 47+++++++++++++++++------------------------------
Mtest/helpers/harness.js | 2+-
Mtest/pipeline.test.js | 9+++++++++
Mtest/retention.test.js | 2+-
Atest/security.test.js | 223+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
23 files changed, 387 insertions(+), 172 deletions(-)

diff --git a/.conductor.yml b/.conductor.yml @@ -7,7 +7,6 @@ # keeps the fast job honest about what it did and did not check. version: 1 -visibility: public defaults: image: node:24-alpine diff --git a/README.md b/README.md @@ -128,15 +128,18 @@ Who sees what ------------- Everything is private by default. A job is visible to an anonymous visitor -only when the project, or the pipeline at that commit, says it is public: +only when its project is public: -```yaml -version: 1 -visibility: public +```sh +curl -X PATCH "$CONDUCTOR/api/v1/projects/demo" \ + -H "authorization: Bearer $TOKEN" \ + -d '{"visibility":"public"}' ``` -Because it is recorded per job, making a repository private stops exposing -future jobs without rewriting the history of past ones. +The setting is copied onto each job as it is created, so turning a project +private stops exposing future jobs without rewriting past ones. A repository +cannot set this: a pipeline that could publish its own results would let +anyone who can land a commit read a private project's logs and artifacts. Users register their own projects and their own workers. A worker someone registers is only ever offered tasks from that person's projects, which is diff --git a/deploy/smoke.sh b/deploy/smoke.sh @@ -71,7 +71,6 @@ log "preparing a repository" mkdir -p "${WORK}/repo" cat > "${WORK}/repo/.conductor.yml" <<'PIPELINE' version: 1 -visibility: public defaults: image: alpine:3 tasks: @@ -162,7 +161,7 @@ compose exec -T conductor git -C /repo rev-parse HEAD >/dev/null \ || fail "the copied repository is not readable inside the conductor" log "registering the project and a worker" -compose exec -T conductor node src/admin-cli.js project:add demo /repo --secret "${SECRET}" >/dev/null +compose exec -T conductor node src/admin-cli.js project:add demo /repo --secret "${SECRET}" --visibility public >/dev/null SMOKE_TOKEN=$(compose exec -T conductor node src/admin-cli.js token:add smoke-worker | awk '/token:/{print $2}') [ -n "${SMOKE_TOKEN}" ] || fail "no worker token was issued" export SMOKE_TOKEN diff --git a/docs/api.md b/docs/api.md @@ -22,7 +22,7 @@ Three kinds of credential reach it, all in `Authorization: Bearer`: | ------------------- | ---------------------------------- | --------------------------- | | access token | the code flow, see `docs/auth.md` | everything a person may do | | worker token | `POST /api/v1/workers` | the worker protocol only | -| project HMAC | a project's trigger secret | that project's jobs | +| project HMAC | a project's trigger secret | starting that project's jobs | An access token is a JWT and a worker token is 64 hex characters, so the two never parse as each other. The project HMAC is not a bearer credential @@ -94,7 +94,8 @@ POST /api/v1/projects/:project/trigger Accepts the payload from `hooks/post-receive`, and the push events of GitHub, Gitea and GitLab. Signed with the project's trigger secret: `X-Hub-Signature-256: sha256=<hmac>` over the exact request body, or -`X-Gitlab-Token`. +`X-Gitlab-Token`. A session that may manage the project is accepted +instead, for callers driving the API rather than forwarding a hook. ```json { "sha": "<40 hex>", "base": "<40 hex or empty>", "ref": "refs/heads/main", "actor": "alice" } @@ -163,9 +164,8 @@ being quietly served. The lists take `?state=` alongside the paging. } ``` -Three ways to be allowed: the job is public, the caller holds the project's -trigger secret, or the caller may manage the project. One credential -therefore covers starting a build and watching it. +Two ways to be allowed: the job is public, or the caller may manage the +project. `/tasks` returns the tasks and the edges between them, which is what a stage graph is drawn from: @@ -205,9 +205,7 @@ GET /api/v1/tasks/:task/artifacts Each also answers under `/api/v1/projects/:project/tasks/:task`, where the project is asserted: a guessed id cannot be read through whichever project -happens to be readable. The task names its own project either way, so a -trigger secret is checked on both. Signing a GET means an HMAC over an -empty body. +happens to be readable. A private task needs a session on both paths. ```json { @@ -278,6 +276,13 @@ GET /api/v1/sessions /sessions/:id DELETE /api/v1/users/me` is who am I. Listing users as a non-administrator returns just themselves rather than a refusal. +Changing your own password takes `current_password` alongside the new one. +An administrator setting someone else's does not, which is what a reset is. + +`repo_url` must be `https`, `ssh` or `git`, or an `scp`-style `user@host:path`. +Other schemes select a git transport rather than a location: `ext::` runs a +command, `file://` reads the conductor's own disk. + Two things are shown exactly once, in the response that creates them, and are not readable afterwards: diff --git a/docs/auth.md b/docs/auth.md @@ -200,6 +200,9 @@ the header cannot be confused. It reaches the worker protocol and nothing else. A project trigger secret is an HMAC over the exact request body, in -`X-Hub-Signature-256`, or `X-Gitlab-Token` compared directly. A project -with no secret accepts unsigned triggers and says so in the log, which is -convenient on a private network and a bad idea anywhere else. +`X-Hub-Signature-256`, or `X-Gitlab-Token` compared directly. It starts +builds on its own project and reaches nothing else. + +Starting a build takes either that signature or a session that may manage +the project. A project whose secret has been cleared is buildable only by +its owner and by administrators, never anonymously. diff --git a/docs/pipeline.md b/docs/pipeline.md @@ -14,7 +14,6 @@ Top level | Key | Required | Description | | ------------ | -------- | --------------------------------------------------- | | `version` | yes | Must be `1`. | -| `visibility` | no | `public` or `private`. Overrides the project. | | `workdir` | no | Absolute path tasks run in. Overrides the project. | | `defaults` | no | Values inherited by every task. | | `tasks` | yes | Mapping of task name to task. | @@ -43,18 +42,14 @@ tasks: Visibility ---------- -Jobs are private unless something says otherwise, and the repository has the -final word on its own results: - -```yaml -version: 1 -visibility: public -``` - -Unset means the project's setting stands. The value is recorded on each job -as it is created, so turning a repository private stops exposing future jobs +Visibility is a project setting, not a pipeline one. It is recorded on each +job as it is created, so turning a project private stops exposing future jobs without retroactively hiding, or revealing, earlier ones. +A `visibility` key here is rejected: a repository that could publish its own +results would let anyone able to land a commit read a private project's logs +and artifacts. Set it with `PATCH /api/v1/projects/:id` instead. + A public job, its logs and its artifacts are readable by anyone. A private job is readable by the project's owner and by administrators. diff --git a/src/admin-cli.js b/src/admin-cli.js @@ -119,8 +119,9 @@ try { repo_url: repoUrl, default_branch: flags.branch ? String(flags.branch) : 'main', config_path: flags.config ? String(flags.config) : '.conductor.yml', + visibility: flags.visibility ? String(flags.visibility) : 'private', trigger_secret: secret, - }); + }, { allowLocalPath: true }); console.log(`created project ${project.id}`); console.log(` repository: ${project.repo_url}`); diff --git a/src/conductor/routes/api.js b/src/conductor/routes/api.js @@ -14,12 +14,25 @@ import taskRoutes from './tasks.js'; import userRoutes from './users.js'; import workerRoutes from './workers.js'; import { envelopeFailures } from './envelope.js'; +import { MalformedJson } from '../../lib/errors.js'; // prefix is dropped rather than passed on: it applied when this plugin was // registered, and handing it to the children would apply it a second time. export default async function apiRoutes(fastify, { prefix, ...services }) { envelopeFailures(fastify); + // Trigger signatures HMAC the raw bytes; registered here so rawBody + // reaches every child plugin. + fastify.addContentTypeParser('application/json', { parseAs: 'buffer' }, (req, body, done) => { + req.rawBody = body; + if (body.length === 0) return done(null, {}); + try { + done(null, JSON.parse(body.toString('utf8'))); + } catch { + done(new MalformedJson()); + } + }); + await fastify.register(jobRoutes, services); await fastify.register(agentRoutes, services); await fastify.register(taskRoutes, services); diff --git a/src/conductor/routes/jobs.js b/src/conductor/routes/jobs.js @@ -1,22 +1,13 @@ -// src/conductor/routes/jobs.js - creating jobs, and reading them back +// src/conductor/routes/jobs.js - job creation and read-back // -// A job is a pipeline run: compiling the repository's .conductor.yml at one -// commit produces the tasks that a worker will later claim. There are two -// ways in, differing only in the shape of what they accept. +// POST /projects/:project/trigger push payload: post-receive, GitHub, Gitea, GitLab +// POST /projects/:project/jobs explicit { sha, base, ref, actor, trigger } // -// POST /projects/:project/trigger takes a push payload. It accepts the -// generic one produced by hooks/post-receive and the push events of -// GitHub, Gitea and GitLab, since they all carry the same three facts. +// Payloads are trusted only for which commit to read; the pipeline always +// comes from the repository at that commit. // -// POST /projects/:project/jobs takes those facts directly, for anything -// driving the conductor deliberately rather than forwarding a hook. -// -// Nothing in either payload is trusted beyond which commit to look at: the -// pipeline is always read from the repository at that commit. -// -// A project with a trigger secret requires a valid signature. A project -// without one accepts unsigned requests, which is convenient on a private -// network and a bad idea anywhere else, so it is logged. +// Create: valid signature, or a session that may manage the project. +// Read: session only. A trigger secret is not a read credential. import { SHA_PATTERN } from '../../lib/git.js'; import { canManageProject } from '../../lib/projects.js'; @@ -31,34 +22,22 @@ import { limitOf, offsetOfQuery, pageBlock, paginateOffset } from '../../lib/pag const ZERO_SHA = '0'.repeat(40); -export default async function jobRoutes(fastify, { cfg, auth, db, projects, scheduler, logger }) { +export default async function jobRoutes(fastify, { cfg, auth, db, projects, scheduler }) { const base = cfg.server.public_url.replace(/\/+$/, ''); + const identify = (req) => auth.identify(req).catch(() => null); - // HMAC is computed over the exact bytes received, so the raw body has to - // survive JSON parsing. - fastify.addContentTypeParser('application/json', { parseAs: 'buffer' }, (req, body, done) => { - req.rawBody = body; - if (body.length === 0) return done(null, {}); - try { - done(null, JSON.parse(body.toString('utf8'))); - } catch { - done(new MalformedJson()); - } - }); - - // Shared by both creation routes: resolve the project, check the secret. + // A signature over the body, or a session that may manage the project. async function authorizedProject(req) { const project = await projects.get(req.params.project); if (!project) throw new UnknownProject(); if (project.enabled !== 1) throw new ProjectDisabled(); const secret = projects.triggerSecret(project); - if (secret) { - if (!verifySignature(req, secret)) throw new InvalidSignature(); - } else { - logger.warn?.(`project ${project.id} accepted an unsigned request; set a trigger secret`); - } - return project; + if (secret && verifySignature(req, secret)) return project; + + if (canManageProject(await identify(req), project)) return project; + + throw new InvalidSignature(); } // Records the job as pending and returns immediately. Pipeline compilation @@ -118,19 +97,9 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche j.trigger_type, j.actor, j.title, j.state, j.visibility, j.error, j.created_at, j.started_at, j.finished_at`; - // Holding the trigger secret is a third way in, so whatever starts a job - // can see how it went without also being a signed in user. - async function mayRead(req, project) { - const secret = projects.triggerSecret(project); - if (secret && verifySignature(req, secret)) return true; - - const user = await auth.identify(req).catch(() => null); - return canManageProject(user, project); - } - async function listJobs(req, reply, { project = null } = {}) { - const user = await auth.identify(req).catch(() => null); - const scope = project && await mayRead(req, project) + const user = await identify(req); + const scope = project && canManageProject(user, project) ? { sql: '1 = 1', params: {} } : visible(user, { project: 'p', job: 'j' }); @@ -175,7 +144,7 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche // Missing rather than refused, so an id cannot be probed by status code. async function readableJob(req) { - const user = await auth.identify(req).catch(() => null); + const user = await identify(req); const job = await db.get( `SELECT ${JOB_COLUMNS} FROM jobs j WHERE j.id = {id}`, { id: req.params.job } @@ -184,7 +153,7 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche if (req.params.project !== undefined && req.params.project !== job.project_id) throw new UnknownJob(); const project = await projects.get(job.project_id); - if (job.visibility !== 'public' && !(await mayRead(req, project))) throw new UnknownJob(); + if (job.visibility !== 'public' && !canManageProject(user, project)) throw new UnknownJob(); return { job, project, user }; } diff --git a/src/conductor/routes/projects.js b/src/conductor/routes/projects.js @@ -46,7 +46,7 @@ export default async function projectRoutes(fastify, { cfg, auth, projects, vari const body = req.body ?? {}; const owner = user.role === 'admin' && Object.hasOwn(body, 'owner_id') ? body.owner_id : user.id; - const secret = body.trigger_secret === null ? null : (body.trigger_secret || newToken()); + const secret = body.trigger_secret || newToken(); let project; try { diff --git a/src/conductor/routes/tasks.js b/src/conductor/routes/tasks.js @@ -1,36 +1,18 @@ // src/conductor/routes/tasks.js - reading a task back // -// A task is the standalone unit a worker runs, and this is how everyone who -// is not that worker looks at one: its state, and the artifacts it stored. -// routes/workers.js holds the protocol the worker itself speaks, behind a -// worker token. These two files share a URL prefix and nothing else, which -// is why they are separate plugins: the token hook over there covers its -// whole scope, and must not reach these routes. -// -// Two paths, one answer: -// // GET /tasks/:task a task id is unique on its own -// GET /projects/:project/tasks/:task the same task, said in full +// GET /projects/:project/tasks/:task the same task, project asserted // -// They differ only in what may reach a private task. A task id is enough to -// prove nothing, so the bare path accepts a public task or a session that -// may manage the project. The scoped path can look up the project's trigger -// secret, so it also accepts a signature, matching the job read-back it sits -// beside: one credential covers starting a build, watching the job and -// reading any task in it. +// The worker protocol lives in routes/agent.js, a separate plugin so its +// worker-token hook cannot reach these routes. // -// Absent and forbidden are both 404, so neither can be used to find out -// which tasks exist. +// A public task reads without a session; a private one needs a session that +// may manage the project. Absent and forbidden are both 404. // -// What is deliberately not here: the task's environment, and its services, -// which carry an environment of their own. Both can hold project variables, -// and a public job is still built from a repository that may be private. The -// script is left out for the same reason. What remains describes what the -// task was and how it went, which is what a caller asking after a task is -// actually asking. +// env, services and script are deliberately omitted: all three can carry +// project variables, and a public job may be built from a private repository. import { canManageProject } from '../../lib/projects.js'; -import { verifySignature } from '../../lib/signature.js'; import { ok } from './envelope.js'; import { UnknownTask } from '../../lib/errors.js'; import { outputAad } from '../../lib/outputs.js'; @@ -126,7 +108,7 @@ export default async function taskRoutes(fastify, { cfg, db, logs, storage, proj if (row.visibility !== 'public') { const project = await projects.get(row.project_id); - if (!(await mayRead(req, project))) throw new UnknownTask(); + if (!canManageProject(await currentUser(req), project)) throw new UnknownTask(); } return row; } @@ -210,13 +192,6 @@ export default async function taskRoutes(fastify, { cfg, db, logs, storage, proj expired: false, }); }); - - async function mayRead(req, project) { - const secret = projects.triggerSecret(project); - if (secret && verifySignature(req, secret)) return true; - - return canManageProject(await currentUser(req), project); - } } // The stored spec, reduced to the keys that are safe to publish. env and diff --git a/src/conductor/routes/users.js b/src/conductor/routes/users.js @@ -86,6 +86,13 @@ export default async function userRoutes(fastify, { cfg, auth, users, refreshTok if (Object.hasOwn(body, 'password')) { const bad = validatePassword(String(body.password ?? '')); if (bad) throw new BadRequest(bad); + + // Keeps a stolen access token from becoming a permanent takeover. + if (user.id === caller.id) { + const proven = await users.authenticate(user.username, String(body.current_password ?? '')); + if (!proven) throw new NotPermitted('current_password does not match'); + } + await users.setPassword(user.id, body.password); await refreshTokens.revokeAllFor(user.id); } diff --git a/src/conductor/scheduler.js b/src/conductor/scheduler.js @@ -170,7 +170,7 @@ export function createScheduler({ const info = await git.commitInfo(project, headSha).catch(() => ({ subject: null })); const now = Date.now(); - const visibility = pipeline.visibility ?? project.visibility ?? 'private'; + const visibility = project.visibility ?? 'private'; const workdir = pipeline.workdir ?? project.workdir ?? DEFAULT_WORKDIR; await db.transaction(async (tx) => { diff --git a/src/lib/git.js b/src/lib/git.js @@ -44,6 +44,10 @@ function gitEnv(env = process.env) { GIT_TERMINAL_PROMPT: '0', GIT_ASKPASS: '', GCM_INTERACTIVE: 'never', + // Backstop for assertRepoUrl: blocks ext:: and every other helper + // transport. file stays for local mirrors, which assertRepoUrl gates. + GIT_ALLOW_PROTOCOL: 'file:git:ssh:https', + GIT_PROTOCOL_FROM_USER: '0', LC_ALL: 'C', GIT_CONFIG_COUNT: String(base + 1), [`GIT_CONFIG_KEY_${base}`]: 'safe.directory', diff --git a/src/lib/pipeline/index.js b/src/lib/pipeline/index.js @@ -16,7 +16,7 @@ export { expandPipeline, interpolate, outputRefsIn, fillOutputs } from './expand export { topologicalOrder, depths, transitiveDependents, runnable, CycleError } from './dag.js'; export { refMatches, taskRunsOnRef, selectForRef } from './only.js'; export { PipelineError } from './schema.js'; -export { SUPPORTED_VERSION, VISIBILITIES, DEFAULT_WORKDIR, parseWorkdir } from './parse.js'; +export { SUPPORTED_VERSION, DEFAULT_WORKDIR, parseWorkdir } from './parse.js'; export function compilePipeline(text, options = {}) { const pipeline = parsePipeline(text, options); @@ -43,7 +43,6 @@ export function compilePipeline(text, options = {}) { return { version: pipeline.version, source: pipeline.source, - visibility: pipeline.visibility, workdir: pipeline.workdir, tasks, }; diff --git a/src/lib/pipeline/parse.js b/src/lib/pipeline/parse.js @@ -22,11 +22,7 @@ import { NAME_PATTERN, } from './schema.js'; -const TOP_KEYS = ['version', 'visibility', 'workdir', 'defaults', 'tasks']; - -// A repository decides whether its own build results are readable without -// signing in. Unset means the project's setting stands. -export const VISIBILITIES = ['public', 'private']; +const TOP_KEYS = ['version', 'workdir', 'defaults', 'tasks']; // Where the tree is put inside a task container, and the directory every // script starts in. Unset means the project's setting stands, and failing @@ -114,6 +110,14 @@ export function parsePipeline(text, options = {}) { ); } + // Named outright; checkUnknown would only offer a nearest match. + if (doc.visibility !== undefined) { + throw new PipelineError( + [{ path: 'visibility', message: 'is no longer set by the pipeline; it is a project setting' }], + source + ); + } + checkUnknown(problems, '', doc, TOP_KEYS); if (doc.version === undefined) { @@ -122,16 +126,6 @@ export function parsePipeline(text, options = {}) { problems.add('version', `unsupported version ${JSON.stringify(doc.version)}, expected ${SUPPORTED_VERSION}`); } - let visibility = null; - if (doc.visibility !== undefined) { - const value = asString(problems, 'visibility', doc.visibility); - if (value !== undefined && !VISIBILITIES.includes(value)) { - problems.add('visibility', `expected one of ${VISIBILITIES.join(', ')}, got ${JSON.stringify(value)}`); - } else if (value !== undefined) { - visibility = value; - } - } - const defaults = parseDefaults(problems, doc.defaults); if (doc.tasks === undefined) { @@ -161,7 +155,7 @@ export function parsePipeline(text, options = {}) { const workdir = doc.workdir === undefined ? null : parseWorkdir(problems, 'workdir', doc.workdir); problems.throwIfAny(source); - return { version: SUPPORTED_VERSION, visibility, workdir, defaults, tasks, source }; + return { version: SUPPORTED_VERSION, workdir, defaults, tasks, source }; } function parseDefaults(problems, raw) { diff --git a/src/lib/projects.js b/src/lib/projects.js @@ -19,6 +19,36 @@ import { Problems } from './pipeline/schema.js'; export const VISIBILITIES = ['public', 'private']; +// A git remote selects a transport: ext:: execs, file:// reads local disk, +// http:// reaches anything routable from the conductor. +const REPO_SCHEMES = new Set(['https:', 'ssh:', 'git:']); +const SCP_LIKE = /^[A-Za-z0-9_.+-]+@[A-Za-z0-9_.-]+:(?!\/)/; + +// allowLocalPath: admin CLI only, which already has host access. +export function assertRepoUrl(value, { allowLocalPath = false } = {}) { + if (typeof value !== 'string' || value.trim().length === 0) { + throw new Error('repo_url is required'); + } + const url = value.trim(); + if (url.startsWith('-')) throw new Error('repo_url must not start with -'); + if (allowLocalPath && url.startsWith('/')) return url; + if (SCP_LIKE.test(url)) return url; + + let parsed; + try { + parsed = new URL(url); + } catch { + throw new Error(`repo_url is not a valid url: ${JSON.stringify(url)}`); + } + if (!REPO_SCHEMES.has(parsed.protocol)) { + throw new Error( + `repo_url scheme ${JSON.stringify(parsed.protocol)} is not allowed; ` + + `use one of ${[...REPO_SCHEMES].join(', ')}` + ); + } + return url; +} + // Validated with the same rule the pipeline uses, so a project and a // repository cannot disagree about what a usable working directory is. export function assertWorkdir(value) { @@ -86,14 +116,14 @@ export function createProjects({ db, secrets }) { }); }, - async create(input) { + async create(input, { allowLocalPath = false } = {}) { const id = input.id ? slugify(input.id) : (slugify(input.name) || newProjectId()); const now = Date.now(); if (input.visibility && !VISIBILITIES.includes(input.visibility)) { throw new Error(`visibility must be one of ${VISIBILITIES.join(', ')}`); } - if (!input.repo_url) throw new Error('repo_url is required'); + const repoUrl = assertRepoUrl(input.repo_url, { allowLocalPath }); if (await this.get(id)) throw new Error(`project ${id} already exists`); @@ -109,7 +139,7 @@ export function createProjects({ db, secrets }) { { id, name: input.name || id, - repo_url: input.repo_url, + repo_url: repoUrl, branch: input.default_branch || 'main', config_path: input.config_path || '.conductor.yml', workdir: input.workdir ? assertWorkdir(input.workdir) : null, diff --git a/test/api.test.js b/test/api.test.js @@ -208,7 +208,7 @@ test('the lists an anonymous reader lands on hold only public rows', async () => repo_url: h.repoDir, visibility: 'private', owner_id: null, - }); + }, { allowLocalPath: true }); const job = data(await h.trigger({ sha: h.sha, ref: 'refs/heads/main' })).job_id; await h.waitForJob(job); @@ -231,7 +231,7 @@ test('a private project is absent rather than refused', async () => { repo_url: h.repoDir, visibility: 'private', owner_id: null, - }); + }, { allowLocalPath: true }); const res = await h.app.inject({ method: 'GET', url: '/api/v1/projects/hidden' }); assert.equal(res.statusCode, 404, res.body); diff --git a/test/conductor.test.js b/test/conductor.test.js @@ -298,22 +298,26 @@ test('a public job is readable without any credential at all', async () => { }); }); -test('a private job needs the trigger secret, and is otherwise absent', async () => { - const pipeline = DEFAULT_PIPELINE.replace('version: 1\n', 'version: 1\nvisibility: private\n'); - await withHarness({ pipeline }, async (h) => { +test('a private job is absent to anyone without a session', async () => { + await withHarness({ visibility: 'private', bootstrap: true }, async (h) => { const jobId = (await h.trigger({ sha: h.sha, ref: 'refs/heads/main' })).json().data.job_id; await h.waitForJob(jobId); const anon = await h.app.inject({ method: 'GET', url: `/api/v1/projects/demo/jobs/${jobId}` }); assert.equal(anon.statusCode, 404); + // The trigger secret starts builds; it does not read them back. const signed = await h.app.inject({ method: 'GET', url: `/api/v1/projects/demo/jobs/${jobId}`, headers: { 'content-type': 'application/json', 'x-hub-signature-256': h.sign('') }, }); - assert.equal(signed.statusCode, 200, signed.body); - assert.equal(signed.json().data.visibility, 'private'); + assert.equal(signed.statusCode, 404, signed.body); + + const { headers } = await h.login(); + const admin = await h.app.inject({ method: 'GET', url: `/api/v1/projects/demo/jobs/${jobId}`, headers }); + assert.equal(admin.statusCode, 200, admin.body); + assert.equal(admin.json().data.visibility, 'private'); }); }); @@ -551,9 +555,8 @@ test('reading a task back needs no worker token', async () => { }); }); -test('a private task is absent without a credential, and needs its project named', async () => { - const pipeline = DEFAULT_PIPELINE.replace('version: 1\n', 'version: 1\nvisibility: private\n'); - await withHarness({ pipeline }, async (h) => { +test('a private task is absent without a session, by either path', async () => { + await withHarness({ visibility: 'private' }, async (h) => { const res = await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); await h.waitForJob(res.json().data.job_id); const task = await claimNamed(h, 'lint', {}); @@ -563,34 +566,18 @@ test('a private task is absent without a credential, and needs its project named assert.equal(anon.statusCode, 404, `${url} should be absent anonymously`); } + // A trigger secret reads nothing, on either path. const signed = { 'content-type': 'application/json', 'x-hub-signature-256': h.sign('') }; - - const scoped = await h.app.inject({ - method: 'GET', - url: `/api/v1/projects/demo/tasks/${task.id}`, - headers: signed, - }); - assert.equal(scoped.statusCode, 200, scoped.body); - assert.equal(scoped.json().data.task.visibility, 'private'); - - // The task names its own project, so the secret is checked either way. - const bare = await h.app.inject({ method: 'GET', url: `/api/v1/tasks/${task.id}`, headers: signed }); - assert.equal(bare.statusCode, 200, bare.body); - - // A signature from another project is still no use. - const wrong = await h.app.inject({ - method: 'GET', - url: `/api/v1/tasks/${task.id}`, - headers: { 'content-type': 'application/json', 'x-hub-signature-256': 'sha256=' + '0'.repeat(64) }, - }); - assert.equal(wrong.statusCode, 404); + for (const url of taskUrls(task.id)) { + const res2 = await h.app.inject({ method: 'GET', url, headers: signed }); + assert.equal(res2.statusCode, 404, `${url} should not accept a trigger secret`); + } }); }); test('a private task is readable by an administrator, by either path', async () => { - const pipeline = DEFAULT_PIPELINE.replace('version: 1\n', 'version: 1\nvisibility: private\n'); // scrypt is slow, so the harness only creates the admin when asked. - await withHarness({ pipeline, bootstrap: true }, async (h) => { + await withHarness({ visibility: 'private', bootstrap: true }, async (h) => { const res = await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); await h.waitForJob(res.json().data.job_id); const task = await claimNamed(h, 'lint', {}); diff --git a/test/helpers/harness.js b/test/helpers/harness.js @@ -153,7 +153,7 @@ export async function startHarness(options = {}) { // which creates private projects explicitly. visibility: options.visibility ?? 'public', owner_id: options.ownerId ?? null, - }); + }, { allowLocalPath: true }); const worker = await services.workerTokens.create('test-worker'); diff --git a/test/pipeline.test.js b/test/pipeline.test.js @@ -695,3 +695,12 @@ test('an unknown expression names task outputs as an option', () => { /expected arch, matrix\.<name> or tasks\.<task>\.outputs\.<name>/ ); }); + +test('the pipeline cannot set visibility; it is a project setting', () => { + for (const value of ['public', 'private']) { + assert.deepEqual( + errorPaths(() => compilePipeline(`version: 1\nvisibility: ${value}\n${MINIMAL.slice(MINIMAL.indexOf('tasks:'))}`)), + ['visibility'] + ); + } +}); diff --git a/test/retention.test.js b/test/retention.test.js @@ -367,7 +367,7 @@ test('one project policy does not reach into another', async () => { repo_url: h.repoDir, trigger_secret: 'other-secret', visibility: 'public', - }); + }, { allowLocalPath: true }); await h.services.db.run( 'UPDATE projects SET artifact_keep_days = 0 WHERE id = {id}', { id: 'other' } ); diff --git a/test/security.test.js b/test/security.test.js @@ -0,0 +1,223 @@ +// test/security.test.js - the authentication boundaries, as regressions +// +// Each test here stands for a specific way in that used to work. + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { startHarness, apiError } from './helpers/harness.js'; +import { assertRepoUrl } from '../src/lib/projects.js'; + +async function withHarness(options, fn) { + const h = await startHarness(options); + try { + await fn(h); + } finally { + await h.stop(); + } +} + +const json = { 'content-type': 'application/json' }; +const PUSH = { sha: null, ref: 'refs/heads/main' }; + +function push(h) { + return JSON.stringify({ ...PUSH, sha: h.sha }); +} + +// --- starting a build --- + +test('a trigger without a signature or a session is refused', async () => { + await withHarness({}, async (h) => { + const res = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects/demo/trigger', + headers: json, + payload: push(h), + }); + assert.equal(res.statusCode, 401, res.body); + assert.equal(apiError(res).name, 'InvalidSignature'); + }); +}); + +test('a project carrying no trigger secret is still not world-buildable', async () => { + await withHarness({}, async (h) => { + await h.services.projects.setTriggerSecret('demo', null); + + for (const url of ['/api/v1/projects/demo/trigger', '/api/v1/projects/demo/jobs']) { + const res = await h.app.inject({ method: 'POST', url, headers: json, payload: push(h) }); + assert.equal(res.statusCode, 401, `${url}: ${res.body}`); + } + }); +}); + +test('a valid webhook signature still starts a build', async () => { + await withHarness({}, async (h) => { + const body = push(h); + const res = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects/demo/trigger', + headers: { ...json, 'x-hub-signature-256': h.sign(body) }, + payload: body, + }); + assert.equal(res.statusCode, 200, res.body); + assert.equal(res.json().data.status, 'created'); + await h.waitForJob(res.json().data.job_id); + }); +}); + +test('an owner may trigger with a session and no signature', async () => { + await withHarness({ bootstrap: true }, async (h) => { + const { headers } = await h.login(); + const res = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects/demo/trigger', + headers: { ...headers, ...json }, + payload: push(h), + }); + assert.equal(res.statusCode, 200, res.body); + await h.waitForJob(res.json().data.job_id); + }); +}); + +// --- reading a build back --- + +test('a trigger secret does not read a private job', async () => { + await withHarness({ visibility: 'private' }, async (h) => { + const body = push(h); + const created = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects/demo/trigger', + headers: { ...json, 'x-hub-signature-256': h.sign(body) }, + payload: body, + }); + const jobId = created.json().data.job_id; + await h.waitForJob(jobId); + + for (const url of [`/api/v1/jobs/${jobId}`, `/api/v1/projects/demo/jobs/${jobId}`]) { + const res = await h.app.inject({ + method: 'GET', + url, + headers: { ...json, 'x-hub-signature-256': h.sign('') }, + }); + assert.equal(res.statusCode, 404, `${url}: ${res.body}`); + } + }); +}); + +// --- repo_url --- + +test('assertRepoUrl refuses transports that are not a repository', async () => { + const refused = [ + "ext::sh -c 'id>/tmp/pwned'", + 'file:///etc/passwd', + 'file:///srv/git/other.git', + 'http://169.254.169.254/latest/meta-data/', + '--upload-pack=/bin/sh', + '/srv/git/other.git', + '', + ]; + for (const url of refused) { + assert.throws(() => assertRepoUrl(url), undefined, `${JSON.stringify(url)} should be refused`); + } + + const accepted = [ + 'https://example.com/a/b.git', + 'ssh://git@example.com/a/b.git', + 'git://example.com/a/b.git', + 'git@example.com:a/b.git', + ]; + for (const url of accepted) { + assert.equal(assertRepoUrl(url), url); + } + + // The admin CLI clones from disk; the API never does. + assert.equal(assertRepoUrl('/srv/git/other.git', { allowLocalPath: true }), '/srv/git/other.git'); +}); + +test('the api refuses a project whose repo_url is a command', async () => { + await withHarness({ bootstrap: true }, async (h) => { + const { headers } = await h.login(); + const res = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects', + headers: { ...headers, ...json }, + payload: JSON.stringify({ id: 'evil', repo_url: "ext::sh -c 'id'" }), + }); + assert.equal(res.statusCode, 400, res.body); + }); +}); + +test('a project is never created without a trigger secret', async () => { + await withHarness({ bootstrap: true }, async (h) => { + const { headers } = await h.login(); + const res = await h.app.inject({ + method: 'POST', + url: '/api/v1/projects', + headers: { ...headers, ...json }, + payload: JSON.stringify({ + id: 'nosecret', + repo_url: 'https://example.com/a/b.git', + trigger_secret: null, + }), + }); + assert.equal(res.statusCode, 201, res.body); + assert.equal(res.json().data.has_trigger_secret, true); + assert.ok(res.json().data.trigger_secret); + }); +}); + +// --- passwords --- + +test('changing your own password needs the current one', async () => { + await withHarness({ bootstrap: true }, async (h) => { + const { headers } = await h.login(); + + const bare = await h.app.inject({ + method: 'PATCH', + url: '/api/v1/users/me', + headers: { ...headers, ...json }, + payload: JSON.stringify({ password: 'a-brand-new-password' }), + }); + assert.equal(bare.statusCode, 403, bare.body); + + const wrong = await h.app.inject({ + method: 'PATCH', + url: '/api/v1/users/me', + headers: { ...headers, ...json }, + payload: JSON.stringify({ password: 'a-brand-new-password', current_password: 'nope' }), + }); + assert.equal(wrong.statusCode, 403, wrong.body); + + const right = await h.app.inject({ + method: 'PATCH', + url: '/api/v1/users/me', + headers: { ...headers, ...json }, + payload: JSON.stringify({ + password: 'a-brand-new-password', + current_password: 'bootstrap-password', + }), + }); + assert.equal(right.statusCode, 200, right.body); + }); +}); + +test('an administrator still resets someone else without their password', async () => { + await withHarness({ bootstrap: true }, async (h) => { + const { headers } = await h.login(); + + const created = await h.app.inject({ + method: 'POST', + url: '/api/v1/users', + headers: { ...headers, ...json }, + payload: JSON.stringify({ username: 'someone', password: 'their-password' }), + }); + assert.equal(created.statusCode, 201, created.body); + + const reset = await h.app.inject({ + method: 'PATCH', + url: `/api/v1/users/${created.json().data.id}`, + headers: { ...headers, ...json }, + payload: JSON.stringify({ password: 'an-imposed-password' }), + }); + assert.equal(reset.statusCode, 200, reset.body); + }); +});