conductor

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

commit fc2da9d9d57c054007700240e2596fafd821f0aa
parent 1b62a6d7410a6f82336fd171c021211792b83992
Author: finwo <finwo@pm.me>
Date:   Mon, 28 Sep 2026 17:54:24 +0200

Let a project's owner read a variable back one at a time

Diffstat:
Mdocs/api.md | 9++++++++-
Mdocs/pipeline.md | 5+++++
Msrc/conductor/routes/projects.js | 17+++++++++++++++--
Msrc/lib/variables.js | 12++++++++++++
Mtest/management.test.js | 27+++++++++++++++++++++++++++
Mtest/ownership.test.js | 92+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mweb/src/screen/project.tsx | 52++++++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 211 insertions(+), 3 deletions(-)

diff --git a/docs/api.md b/docs/api.md @@ -302,14 +302,21 @@ forever: PATCH /api/v1/projects/:id { "artifact_keep_days": null, "log_keep_days": 0 } ``` -Variables are per project, and a value is never read back: +Variables are per project. The listing carries names and flags only; a value +comes back one at a time, from its own endpoint, and every read is logged: ``` GET /api/v1/projects/:id/variables +GET /api/v1/projects/:id/variables/:name/value PUT /api/v1/projects/:id/variables/:name { "value": "...", "masked": true } DELETE /api/v1/projects/:id/variables/:name ``` +All four are restricted to the project's owner and to administrators, so +being able to see a public project does not mean being able to read its +credentials. `masked` decides whether a value is redacted from job logs; it +does not decide who may read it. + A worker's machines and its recent tasks hang off it: ``` diff --git a/docs/pipeline.md b/docs/pipeline.md @@ -296,6 +296,11 @@ shows as `[masked]` in the UI. Both kinds are encrypted at rest, exactly as project variables are; `public` decides what may be shown again, not how it is stored. +Masking is stricter here than it is for a project variable. A commit decides +what an output says, so a masked output is not readable back by anyone. A +project variable is set by the people who own the project, and they can read +it back. + Values are stored per task, and a value is only reachable through a reference to the task that produced it, so two tasks may use the same output name without colliding. A reference may be read anywhere `${{ }}` works except diff --git a/src/conductor/routes/projects.js b/src/conductor/routes/projects.js @@ -2,7 +2,7 @@ import { ok } from './envelope.js'; import { identifies, requireUser, requireAdmin } from './guard.js'; -import { BadRequest, Conflict, NotFound, NotPermitted } from '../../lib/errors.js'; +import { BadRequest, Conflict, InternalError, NotFound, NotPermitted } from '../../lib/errors.js'; import { canManageProject, canViewProject, VISIBILITIES } from '../../lib/projects.js'; import { limitOf, offsetOfQuery, pageBlock } from '../../lib/page.js'; import { newToken } from '../../lib/ids.js'; @@ -14,7 +14,7 @@ function present(project) { return { ...rest, has_trigger_secret: secret !== null }; } -export default async function projectRoutes(fastify, { cfg, auth, projects, variables }) { +export default async function projectRoutes(fastify, { cfg, auth, projects, variables, logger = console }) { const base = cfg.server.public_url.replace(/\/+$/, ''); identifies(fastify, auth); @@ -109,6 +109,19 @@ export default async function projectRoutes(fastify, { cfg, auth, projects, vari return ok(reply, await variables.list(project.id)); }); + fastify.get('/projects/:id/variables/:name/value', async (req, reply) => { + const project = await manageable(req); + let revealed; + try { + revealed = await variables.reveal(project.id, req.params.name); + } catch (e) { + throw new InternalError(e.message); + } + if (!revealed) throw new NotFound(); + logger.info?.(`${req.user.username} read variable ${revealed.name} of project ${project.id}`); + return ok(reply, revealed); + }); + fastify.put('/projects/:id/variables/:name', async (req, reply) => { const project = await manageable(req); const body = req.body ?? {}; diff --git a/src/lib/variables.js b/src/lib/variables.js @@ -40,6 +40,18 @@ export function createVariables({ db, secrets, ttl = 30000 }) { })); }, + // Unlike resolve, a value that cannot be opened throws rather than + // reading as absent, or a rotated key would look like a missing variable. + async reveal(projectId, name) { + const row = await db.get( + `SELECT project_id, name, value, masked FROM project_variables + WHERE project_id = {project} AND name = {name}`, + { project: projectId, name } + ); + if (!row) return null; + return { name: row.name, masked: row.masked === 1, value: open(row) }; + }, + async set(projectId, name, value, { masked = true } = {}) { if (!NAME_PATTERN.test(String(name))) { throw new Error(`variable name ${JSON.stringify(name)} is not a valid environment variable name`); diff --git a/test/management.test.js b/test/management.test.js @@ -155,6 +155,33 @@ test('a variable is encrypted at rest and bound to its project and name', async }); }); +test('a value that will not open is reported as broken rather than absent', async () => { + await withAdmin({}, async (h, admin) => { + await h.app.inject({ + method: 'PUT', + url: '/api/v1/projects/demo/variables/TOKEN', + headers: json(admin), + payload: JSON.stringify({ value: 'unopenable-secret-value' }), + }); + + const row = await h.services.db.get( + 'SELECT value FROM project_variables WHERE project_id = {p} AND name = {n}', + { p: 'demo', n: 'TOKEN' } + ); + await h.services.db.run( + `INSERT INTO project_variables (project_id, name, value, masked, created_at) + VALUES ({p}, {n}, {v}, 1, {t})`, + { p: 'demo', n: 'MOVED', v: row.value, t: Date.now() } + ); + + const res = await h.app.inject({ + url: '/api/v1/projects/demo/variables/MOVED/value', headers: admin, + }); + assert.equal(res.statusCode, 500, res.body); + assert.ok(!res.body.includes('unopenable-secret-value')); + }); +}); + test('a pipeline setting wins over a project variable of the same name', async () => { const pipeline = ` version: 1 diff --git a/test/ownership.test.js b/test/ownership.test.js @@ -216,3 +216,95 @@ test('a worker token does not outlive its owner and become shared capacity', asy assert.equal(apiError(claimed).name, 'InvalidWorkerToken'); }); }); + +const valueUrl = (project, name) => `/api/v1/projects/${project}/variables/${name}/value`; + +test('an owner reads back a variable whether or not it is masked in logs', async () => { + await withHarness({}, async (h) => { + const { headers: admin } = await h.login(); + const alice = await addUser(h, admin, 'alice'); + await h.services.projects.setOwner('demo', alice.user.id); + + await h.services.variables.set('demo', 'HIDDEN', 'hidden-value', { masked: true }); + await h.services.variables.set('demo', 'SHOWN', 'shown-value', { masked: false }); + + const hidden = await h.app.inject({ url: valueUrl('demo', 'HIDDEN'), headers: alice.headers }); + assert.equal(hidden.statusCode, 200, hidden.body); + assert.equal(data(hidden).value, 'hidden-value'); + assert.equal(data(hidden).masked, true); + + const shown = await h.app.inject({ url: valueUrl('demo', 'SHOWN'), headers: alice.headers }); + assert.equal(data(shown).value, 'shown-value'); + assert.equal(data(shown).masked, false); + }); +}); + +test('the listing still withholds what the value endpoint hands over', async () => { + await withHarness({}, async (h) => { + const { headers: admin } = await h.login(); + await h.services.variables.set('demo', 'TOKEN', 'listing-secret-value', { masked: true }); + + const listed = await h.app.inject({ url: '/api/v1/projects/demo/variables', headers: admin }); + assert.ok(!listed.body.includes('listing-secret-value')); + assert.equal(data(listed)[0].name, 'TOKEN'); + + const revealed = await h.app.inject({ url: valueUrl('demo', 'TOKEN'), headers: admin }); + assert.equal(data(revealed).value, 'listing-secret-value'); + }); +}); + +test('an administrator reads a variable of a project they do not own', async () => { + await withHarness({}, async (h) => { + const { headers: admin } = await h.login(); + const alice = await addUser(h, admin, 'alice'); + await h.services.projects.setOwner('demo', alice.user.id); + await h.services.variables.set('demo', 'TOKEN', 'admin-readable-value'); + + const res = await h.app.inject({ url: valueUrl('demo', 'TOKEN'), headers: admin }); + assert.equal(res.statusCode, 200, res.body); + assert.equal(data(res).value, 'admin-readable-value'); + }); +}); + +test('a public project does not hand its variables to every signed in user', async () => { + await withHarness({ visibility: 'public' }, async (h) => { + const { headers: admin } = await h.login(); + const alice = await addUser(h, admin, 'alice'); + const bob = await addUser(h, admin, 'bob'); + await h.services.projects.setOwner('demo', alice.user.id); + await h.services.variables.set('demo', 'TOKEN', 'not-for-bob-value'); + + const refused = await h.app.inject({ url: valueUrl('demo', 'TOKEN'), headers: bob.headers }); + assert.equal(refused.statusCode, 403, refused.body); + assert.equal(apiError(refused).name, 'NotPermitted'); + assert.ok(!refused.body.includes('not-for-bob-value')); + + const anonymous = await h.app.inject({ url: valueUrl('demo', 'TOKEN') }); + assert.equal(anonymous.statusCode, 403, anonymous.body); + assert.ok(!anonymous.body.includes('not-for-bob-value')); + }); +}); + +test('a private project answers a variable read as though it were not there', async () => { + await withHarness({ visibility: 'private' }, async (h) => { + const { headers: admin } = await h.login(); + const alice = await addUser(h, admin, 'alice'); + const bob = await addUser(h, admin, 'bob'); + await h.services.projects.setOwner('demo', alice.user.id); + await h.services.variables.set('demo', 'TOKEN', 'not-for-bob-value'); + + const stranger = await h.app.inject({ url: valueUrl('demo', 'TOKEN'), headers: bob.headers }); + assert.equal(stranger.statusCode, 404, stranger.body); + + const anonymous = await h.app.inject({ url: valueUrl('demo', 'TOKEN') }); + assert.equal(anonymous.statusCode, 404, anonymous.body); + }); +}); + +test('reading a variable that was never set is a miss, not an empty value', async () => { + await withHarness({}, async (h) => { + const { headers: admin } = await h.login(); + const res = await h.app.inject({ url: valueUrl('demo', 'ABSENT'), headers: admin }); + assert.equal(res.statusCode, 404, res.body); + }); +}); diff --git a/web/src/screen/project.tsx b/web/src/screen/project.tsx @@ -23,11 +23,15 @@ export const Project: m.Component = { vnode.state.gone = false; vnode.state.secret = null; vnode.state.newVar = { name: '', value: '', masked: true }; + vnode.state.values = {}; + vnode.state.shown = {}; vnode.state.load = async () => { try { vnode.state.project = await get<any>(`/projects/${id}`); vnode.state.jobs = (await page<any>(`/projects/${id}/jobs`, { query: { limit: 10 } })).rows; + vnode.state.values = {}; + vnode.state.shown = {}; vnode.state.variables = manages(vnode.state.project) ? await get<any[]>(`/projects/${id}/variables`) : null; @@ -76,6 +80,39 @@ export const Project: m.Component = { } }; + const valueOf = async (name: string): Promise<string> => { + if (vnode.state.values[name] === undefined) { + const got = await get<any>(`/projects/${id}/variables/${encodeURIComponent(name)}/value`); + vnode.state.values[name] = got.value; + } + return vnode.state.values[name]; + }; + + const toggleValue = async (name: string) => { + if (vnode.state.shown[name]) { + vnode.state.shown[name] = false; + return; + } + try { + await valueOf(name); + vnode.state.shown[name] = true; + } catch (e) { + vnode.state.error = e; + } + }; + + const copyValue = async (name: string) => { + try { + const value = await valueOf(name); + // Undefined outside a secure context, so show the value instead of + // failing on a plain http deployment. + if (!navigator.clipboard) vnode.state.shown[name] = true; + else await navigator.clipboard.writeText(value); + } catch (e) { + vnode.state.error = e; + } + }; + const retention = (key: string, label: string) => ( <label> <span>{label}</span> @@ -154,12 +191,27 @@ export const Project: m.Component = { <h2>variables</h2> {vnode.state.variables.length === 0 ? <Empty what="variables" /> : ( <table> + <thead> + <tr> + <th>name</th> + <th>in logs</th> + <th>value</th> + <th /> + </tr> + </thead> <tbody> {vnode.state.variables.map((v: any) => ( <tr key={v.name}> <td class="mono">{v.name}</td> <td>{v.masked ? 'masked' : 'visible'}</td> + <td class="mono"> + {vnode.state.shown[v.name] ? vnode.state.values[v.name] : '***'} + </td> <td> + <button class="link" onclick={() => toggleValue(v.name)}> + {vnode.state.shown[v.name] ? 'hide' : 'view'} + </button> + <button class="link" onclick={() => copyValue(v.name)}>copy</button> <button class="link" onclick={async () => {