commit d67119ee8275283a621c55d599faf4e253c0c496
parent d91516ad05dac0531f691cd42361a3ce5dadd915
Author: finwo <finwo@pm.me>
Date: Sun, 27 Sep 2026 18:36:08 +0200
Answer the same way for a private project as for one that is not there
Diffstat:
7 files changed, 67 insertions(+), 90 deletions(-)
diff --git a/src/conductor/routes/artifacts.js b/src/conductor/routes/artifacts.js
@@ -1,40 +1,18 @@
// src/conductor/routes/artifacts.js - artifact downloads
//
-// The one read endpoint that is not the interface: the task page links here,
-// so a browser can download build output without the conductor proxying the
-// bytes when the store can presign them.
-//
-// Visibility matches the interface: a public job is downloadable by anyone,
-// a private one by its owner or an administrator. The project, job and task
-// in the path must all agree with the artifact, so a guessed identifier
-// cannot reach anything.
+// The project, job and task in the path must all agree with the artifact, so
+// a guessed identifier cannot reach anything.
import { StorageNotFound } from '../../lib/storage/index.js';
import { ArtifactNotStored, UnknownArtifact } from '../../lib/errors.js';
+import { visibleSql } from '../../lib/visibility.js';
+import { identifies } from './guard.js';
export default async function artifactRoutes(fastify, { db, storage, auth = null }) {
- async function currentUser(req) {
- if (!auth) return null;
- try {
- return await auth.identify(req);
- } catch {
- return null;
- }
- }
-
- function scope(user) {
- if (user && user.role === 'admin') return { sql: '1 = 1', params: {} };
- if (user) {
- return {
- sql: "(j.visibility = 'public' OR p.owner_id = {viewerId})",
- params: { viewerId: user.id },
- };
- }
- return { sql: "j.visibility = 'public'", params: {} };
- }
+ identifies(fastify, auth);
fastify.get('/projects/:project/jobs/:job/tasks/:task/artifacts/:artifact', async (req, reply) => {
- const visible = scope(await currentUser(req));
+ const visible = visibleSql(req.user, { visibility: 'j.visibility', owner: 'p.owner_id' });
const artifact = await db.get(
`SELECT a.id, a.path, a.storage_key, a.size
FROM artifacts a
@@ -43,7 +21,7 @@ export default async function artifactRoutes(fastify, { db, storage, auth = null
JOIN projects p ON p.id = j.project_id
WHERE a.id = {id}
AND a.task_id = {task} AND a.job_id = {job} AND j.project_id = {project}
- AND ${visible.sql}`,
+ AND ${visible.where}`,
{
id: req.params.artifact,
task: req.params.task,
diff --git a/src/conductor/routes/guard.js b/src/conductor/routes/guard.js
@@ -19,16 +19,3 @@ export function requireAdmin(req) {
if (user.role !== 'admin') throw new NotPermitted();
return user;
}
-
-// In SQL rather than in javascript, so a count and its rows cannot disagree.
-export function visible(user, { project = 'p', job = null } = {}) {
- const column = job ? `${job}.visibility` : `${project}.visibility`;
- if (user?.role === 'admin') return { sql: '1 = 1', params: {} };
- if (user) {
- return {
- sql: `(${column} = 'public' OR ${project}.owner_id = {viewerId})`,
- params: { viewerId: user.id },
- };
- }
- return { sql: `${column} = 'public'`, params: {} };
-}
diff --git a/src/conductor/routes/jobs.js b/src/conductor/routes/jobs.js
@@ -10,10 +10,11 @@
// Read: session only. A trigger secret is not a read credential.
import { SHA_PATTERN } from '../../lib/git.js';
-import { canManageProject } from '../../lib/projects.js';
+import { canManageProject, canViewProject } from '../../lib/projects.js';
import { verifySignature } from '../../lib/signature.js';
+import { visibleSql } from '../../lib/visibility.js';
import { ok } from './envelope.js';
-import { visible } from './guard.js';
+import { identifies } from './guard.js';
import {
Conflict, InvalidSha, InvalidSignature, MalformedJson, NoCommitInPayload,
NotPermitted, ProjectDisabled, ShaRequired, UnknownJob, UnknownProject,
@@ -24,7 +25,7 @@ const ZERO_SHA = '0'.repeat(40);
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);
+ identifies(fastify, auth);
// A signature over the body, or a session that may manage the project.
async function authorizedProject(req) {
@@ -35,7 +36,7 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche
const secret = projects.triggerSecret(project);
if (secret && verifySignature(req, secret)) return project;
- if (canManageProject(await identify(req), project)) return project;
+ if (canManageProject(req.user, project)) return project;
throw new InvalidSignature();
}
@@ -98,12 +99,9 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche
j.created_at, j.started_at, j.finished_at`;
async function listJobs(req, reply, { project = null } = {}) {
- const user = await identify(req);
- const scope = project && canManageProject(user, project)
- ? { sql: '1 = 1', params: {} }
- : visible(user, { project: 'p', job: 'j' });
+ const scope = visibleSql(req.user, { visibility: 'j.visibility', owner: 'p.owner_id' });
- const filters = [scope.sql];
+ const filters = [scope.where];
const params = { ...scope.params };
if (project) {
filters.push('j.project_id = {project}');
@@ -129,9 +127,11 @@ export default async function jobRoutes(fastify, { cfg, auth, db, projects, sche
fastify.get('/jobs', async (req, reply) => listJobs(req, reply));
+ // Naming a project you may not see answers the same as naming one that is
+ // not there, or the list would confirm it exists.
fastify.get('/projects/:project/jobs', async (req, reply) => {
const project = await projects.get(req.params.project);
- if (!project) throw new UnknownProject();
+ if (!canViewProject(req.user, project)) throw new UnknownProject();
return listJobs(req, reply, { project });
});
@@ -144,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 identify(req);
+ const user = req.user;
const job = await db.get(
`SELECT ${JOB_COLUMNS} FROM jobs j WHERE j.id = {id}`,
{ id: req.params.job }
diff --git a/src/conductor/routes/tasks.js b/src/conductor/routes/tasks.js
@@ -14,6 +14,7 @@
import { canManageProject } from '../../lib/projects.js';
import { ok } from './envelope.js';
+import { identifies } from './guard.js';
import { UnknownTask } from '../../lib/errors.js';
import { outputAad } from '../../lib/outputs.js';
import { limitOf, offsetOfQuery, pageBlock, paginateOffset } from '../../lib/page.js';
@@ -36,14 +37,7 @@ export default async function taskRoutes(fastify, { cfg, db, logs, storage, proj
FROM tasks t
JOIN jobs j ON j.id = t.job_id`;
- async function currentUser(req) {
- if (!auth) return null;
- try {
- return await auth.identify(req);
- } catch {
- return null;
- }
- }
+ identifies(fastify, auth);
// Names always, values only where the task said they could be shown.
function readOutput(row) {
@@ -108,7 +102,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 (!canManageProject(await currentUser(req), project)) throw new UnknownTask();
+ if (!canManageProject(req.user, project)) throw new UnknownTask();
}
return row;
}
diff --git a/src/lib/projects.js b/src/lib/projects.js
@@ -16,6 +16,7 @@ import { newProjectId, slugify } from './ids.js';
import { paginateOffset } from './page.js';
import { parseWorkdir } from './pipeline/parse.js';
import { Problems } from './pipeline/schema.js';
+import { canManage, canView, visibleSql } from './visibility.js';
export const VISIBILITIES = ['public', 'private'];
@@ -81,36 +82,16 @@ export function createProjects({ db, secrets }) {
return db.all(`SELECT ${COLUMNS} FROM projects ORDER BY name`);
},
- // What a given user is allowed to see. Administrators see everything;
- // everyone else sees what they own plus anything public.
- async listVisible(user) {
- if (user && user.role === 'admin') return this.list();
- if (user) {
- return db.all(
- `SELECT ${COLUMNS} FROM projects
- WHERE owner_id = {owner} OR visibility = 'public'
- ORDER BY name`,
- { owner: user.id }
- );
- }
- return db.all(`SELECT ${COLUMNS} FROM projects WHERE visibility = 'public' ORDER BY name`);
- },
-
async listOwnedBy(ownerId) {
return db.all(`SELECT ${COLUMNS} FROM projects WHERE owner_id = {owner} ORDER BY name`, { owner: ownerId });
},
- // One page of what this user may see: everything for an administrator,
- // their own plus anything public otherwise.
async pageVisible(user, { offset, limit }) {
- const admin = user && user.role === 'admin';
- const mine = user ? "(owner_id = {owner} OR visibility = 'public')" : "visibility = 'public'";
return paginateOffset(db, {
select: COLUMNS,
from: 'projects',
- where: admin ? '1 = 1' : mine,
+ ...visibleSql(user, { visibility: 'visibility', owner: 'owner_id' }),
order: 'name',
- params: user && !admin ? { owner: user.id } : {},
offset,
limit,
});
@@ -242,16 +223,12 @@ export function createProjects({ db, secrets }) {
};
}
-// An unowned project is administered centrally; an owned one is the
-// owner's, and administrators may act on either.
export function canManageProject(user, project) {
- if (!user || !project) return false;
- if (user.role === 'admin') return true;
- return project.owner_id !== null && project.owner_id === user.id;
+ if (!project) return false;
+ return canManage(user, project.owner_id);
}
export function canViewProject(user, project) {
if (!project) return false;
- if (project.visibility === 'public') return true;
- return canManageProject(user, project);
+ return canView(user, { visibility: project.visibility, ownerId: project.owner_id });
}
diff --git a/src/lib/visibility.js b/src/lib/visibility.js
@@ -0,0 +1,25 @@
+// src/lib/visibility.js - who may see a row
+
+// SQL for lists, so a count and its page cannot disagree.
+export function visibleSql(user, { visibility, owner }) {
+ if (user?.role === 'admin') return { where: '1 = 1', params: {} };
+ if (user) {
+ return {
+ where: `(${visibility} = 'public' OR ${owner} = {viewerId})`,
+ params: { viewerId: user.id },
+ };
+ }
+ return { where: `${visibility} = 'public'`, params: {} };
+}
+
+// A null owner matches nobody: an unowned row is administered centrally.
+export function canManage(user, ownerId) {
+ if (!user) return false;
+ if (user.role === 'admin') return true;
+ return ownerId != null && ownerId === user.id;
+}
+
+export function canView(user, { visibility, ownerId }) {
+ if (visibility === 'public') return true;
+ return canManage(user, ownerId);
+}
diff --git a/test/security.test.js b/test/security.test.js
@@ -103,6 +103,22 @@ test('a trigger secret does not read a private job', async () => {
});
});
+test('listing the jobs of a private project does not confirm it exists', async () => {
+ await withHarness({ visibility: 'private', bootstrap: true }, async (h) => {
+ const missing = await h.app.inject({ method: 'GET', url: '/api/v1/projects/nope/jobs' });
+ const hidden = await h.app.inject({ method: 'GET', url: '/api/v1/projects/demo/jobs' });
+
+ assert.equal(hidden.statusCode, 404, hidden.body);
+ assert.equal(apiError(hidden).name, 'UnknownProject');
+ assert.equal(hidden.statusCode, missing.statusCode);
+ assert.equal(apiError(hidden).name, apiError(missing).name);
+
+ const { headers } = await h.login();
+ const seen = await h.app.inject({ method: 'GET', url: '/api/v1/projects/demo/jobs', headers });
+ assert.equal(seen.statusCode, 200, seen.body);
+ });
+});
+
// --- repo_url ---
test('assertRepoUrl refuses transports that are not a repository', async () => {
