fix(mam-api): close cross-project authz gaps in assets/bins/jobs/upload
Review of the v2 auth landing found four places where the per-project RBAC
helpers weren't applied to destination/source projects, letting a scoped
editor write into projects they don't have access to:
- assets PATCH /🆔 bin_id moved with no check, so an editor in project A
could stuff their asset into a bin in project B. Now validates the bin's
project_id matches the asset's own project (assets don't change project).
- assets POST /:id/copy: body's projectId/binId never checked, so any
reachable asset could be cloned into an arbitrary project. Now asserts
edit on the destination project and validates binId belongs there.
- bins POST /:id/assets: requireBinEdit checks edit on the bin's project but
not on the source asset's project, so an asset from project B could be
pulled into A's bin tree (and surfaced in A's views). Now the asset must
belong to the bin's own project.
- jobs POST /conform: project_id from body never gated, so any logged-in
user could enqueue conform jobs against any project. Now asserts edit.
- upload POST /init, POST /simple: projectId/binId from body never gated,
same class of bug. Now asserts edit on projectId and validates binId.
- upload GET /: returned every in-progress upload globally, leaking
filenames across projects. Now scoped via accessibleProjectIds.
These are the same pattern as the holes 2615143 closed on recorders/
sequences/imports/comments — these routes existed before the RBAC commit
landed and were never marked TODO(authz), so the broad sweep missed them.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
2615143c6d
commit
3fe7d6bba2
4 changed files with 87 additions and 14 deletions
|
|
@ -320,6 +320,18 @@ router.patch('/:id', requireAssetEdit, async (req, res, next) => {
|
|||
try {
|
||||
const { id } = req.params;
|
||||
const { display_name, tags, notes, bin_id } = req.body;
|
||||
|
||||
// bin_id must reference a bin in the asset's OWN project — otherwise an
|
||||
// editor in project A could stuff their asset into project B's bin tree.
|
||||
// Null/empty clears the bin, which is always allowed.
|
||||
if (bin_id) {
|
||||
const bin = await pool.query('SELECT project_id FROM bins WHERE id = $1', [bin_id]);
|
||||
if (bin.rows.length === 0) return res.status(400).json({ error: 'bin_id not found' });
|
||||
if (bin.rows[0].project_id !== req.assetProjectId) {
|
||||
return res.status(400).json({ error: 'bin_id belongs to a different project' });
|
||||
}
|
||||
}
|
||||
|
||||
const updates = [], params = [];
|
||||
let paramCount = 1;
|
||||
if (display_name !== undefined) { updates.push(`display_name = $${paramCount++}`); params.push(display_name); }
|
||||
|
|
@ -345,6 +357,25 @@ router.post('/:id/copy', requireAssetEdit, async (req, res, next) => {
|
|||
const r = await pool.query('SELECT * FROM assets WHERE id = $1', [id]);
|
||||
if (r.rows.length === 0) return res.status(404).json({ error: 'Asset not found' });
|
||||
const src = r.rows[0];
|
||||
|
||||
// Destination project defaults to source's. If the caller overrides it,
|
||||
// assert edit on the target — without this, an editor in project A could
|
||||
// clone any asset they can see into project B with no grant on B.
|
||||
const destProjectId = projectId || src.project_id;
|
||||
if (projectId && projectId !== src.project_id) {
|
||||
await assertProjectAccess(req.user, destProjectId, 'edit');
|
||||
}
|
||||
// Destination bin (if any) must belong to the destination project — same
|
||||
// class of bug as the PATCH bin_id hole.
|
||||
const destBinId = binId === undefined ? src.bin_id : (binId || null);
|
||||
if (destBinId) {
|
||||
const bin = await pool.query('SELECT project_id FROM bins WHERE id = $1', [destBinId]);
|
||||
if (bin.rows.length === 0) return res.status(400).json({ error: 'binId not found' });
|
||||
if (bin.rows[0].project_id !== destProjectId) {
|
||||
return res.status(400).json({ error: 'binId belongs to a different project than the destination' });
|
||||
}
|
||||
}
|
||||
|
||||
const newId = uuidv4();
|
||||
// Bug #60: null out proxy_s3_key and thumbnail_s3_key on the copy to avoid
|
||||
// sharing S3 objects with the source. Set status to 'processing' so the copy
|
||||
|
|
@ -359,8 +390,8 @@ router.post('/:id/copy', requireAssetEdit, async (req, res, next) => {
|
|||
$1,$2,$3,$4,$5,$6,$7,$8,NULL,NULL,$9,$10,$11,$12,$13,$14,$15,$16,NOW(),NOW()
|
||||
) RETURNING *`,
|
||||
[
|
||||
newId, projectId || src.project_id,
|
||||
binId === undefined ? src.bin_id : (binId || null),
|
||||
newId, destProjectId,
|
||||
destBinId,
|
||||
src.filename, src.display_name, 'processing', src.media_type,
|
||||
src.original_s3_key,
|
||||
src.codec, src.resolution, src.fps, src.duration_ms, src.start_tc,
|
||||
|
|
|
|||
|
|
@ -171,10 +171,13 @@ router.post('/:id/assets', requireBinEdit, async (req, res, next) => {
|
|||
return res.status(400).json({ error: 'asset_id is required' });
|
||||
}
|
||||
|
||||
// Verify bin exists
|
||||
const binCheck = await pool.query('SELECT id FROM bins WHERE id = $1', [id]);
|
||||
if (binCheck.rows.length === 0) {
|
||||
return res.status(404).json({ error: 'Bin not found' });
|
||||
// Asset must live in the bin's own project. Without this, an editor in
|
||||
// project A (where the bin lives) could pull an asset from project B (no
|
||||
// grant) into A's bin tree, exposing it in A's views.
|
||||
const a = await pool.query('SELECT project_id FROM assets WHERE id = $1', [asset_id]);
|
||||
if (a.rows.length === 0) return res.status(404).json({ error: 'Asset not found' });
|
||||
if (a.rows[0].project_id !== req.binProjectId) {
|
||||
return res.status(400).json({ error: 'asset belongs to a different project than the bin' });
|
||||
}
|
||||
|
||||
// Update asset's bin_id
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import express from 'express';
|
||||
import pool from '../db/pool.js';
|
||||
import { Queue } from 'bullmq';
|
||||
import { assertProjectAccess } from '../auth/authz.js';
|
||||
|
||||
const router = express.Router();
|
||||
// Note: jobs use BullMQ id format "<queueType>:<bullId>" (e.g. "conform:42"),
|
||||
|
|
@ -324,6 +325,10 @@ router.post('/conform', async (req, res, next) => {
|
|||
});
|
||||
}
|
||||
|
||||
// Conform writes back into a project — require edit on that project. Without
|
||||
// this, any logged-in user could enqueue conform jobs targeting any project.
|
||||
await assertProjectAccess(req.user, project_id, 'edit');
|
||||
|
||||
const bullJob = await conformQueue.add('conform-task', {
|
||||
edl,
|
||||
projectId: project_id,
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
AbortMultipartUploadCommand,
|
||||
} from '@aws-sdk/client-s3';
|
||||
import { getAmppConfig, ensureFolderPath } from '../ampp/client.js';
|
||||
import { assertProjectAccess, accessibleProjectIds } from '../auth/authz.js';
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
|
|
@ -138,16 +139,24 @@ function mediaTypeFromMime(mime = '') {
|
|||
return 'document';
|
||||
}
|
||||
|
||||
// GET /api/v1/upload - List in-progress uploads (#68)
|
||||
// GET /api/v1/upload - List in-progress uploads (#68). Scoped to projects the
|
||||
// caller can see — admins are unfiltered; a scoped viewer/editor only sees
|
||||
// uploads for projects they have access to (no enumeration of other projects'
|
||||
// in-flight filenames).
|
||||
router.get('/', async (req, res, next) => {
|
||||
try {
|
||||
const result = await pool.query(
|
||||
`SELECT id, filename, display_name, project_id, bin_id, status, created_at, updated_at
|
||||
FROM assets
|
||||
WHERE status = 'ingesting'
|
||||
ORDER BY created_at DESC
|
||||
LIMIT 50`
|
||||
);
|
||||
const access = await accessibleProjectIds(req.user);
|
||||
let query = `SELECT id, filename, display_name, project_id, bin_id, status, created_at, updated_at
|
||||
FROM assets
|
||||
WHERE status = 'ingesting'`;
|
||||
const params = [];
|
||||
if (!access.all) {
|
||||
if (access.ids.size === 0) return res.json([]);
|
||||
query += ` AND project_id = ANY($1::uuid[])`;
|
||||
params.push([...access.ids]);
|
||||
}
|
||||
query += ` ORDER BY created_at DESC LIMIT 50`;
|
||||
const result = await pool.query(query, params);
|
||||
res.json(result.rows);
|
||||
} catch (err) { next(err); }
|
||||
});
|
||||
|
|
@ -163,6 +172,17 @@ router.post('/init', async (req, res, next) => {
|
|||
});
|
||||
}
|
||||
|
||||
// Uploading creates an asset under a project — require edit on that project.
|
||||
// Without this, any logged-in user could write into any project.
|
||||
await assertProjectAccess(req.user, projectId, 'edit');
|
||||
if (binId) {
|
||||
const bin = await pool.query('SELECT project_id FROM bins WHERE id = $1', [binId]);
|
||||
if (bin.rows.length === 0) return res.status(400).json({ error: 'binId not found' });
|
||||
if (bin.rows[0].project_id !== projectId) {
|
||||
return res.status(400).json({ error: 'binId belongs to a different project' });
|
||||
}
|
||||
}
|
||||
|
||||
const assetId = uuidv4();
|
||||
const s3Key = `originals/${assetId}/${filename}`;
|
||||
const tagsArray = tags ? (Array.isArray(tags) ? tags : [tags]) : [];
|
||||
|
|
@ -326,6 +346,20 @@ router.post('/simple', upload.single('file'), async (req, res, next) => {
|
|||
});
|
||||
}
|
||||
|
||||
// Same authz gate as /init.
|
||||
await assertProjectAccess(req.user, projectId, 'edit');
|
||||
if (binId) {
|
||||
const bin = await pool.query('SELECT project_id FROM bins WHERE id = $1', [binId]);
|
||||
if (bin.rows.length === 0) {
|
||||
unlinkPart(tmpPath);
|
||||
return res.status(400).json({ error: 'binId not found' });
|
||||
}
|
||||
if (bin.rows[0].project_id !== projectId) {
|
||||
unlinkPart(tmpPath);
|
||||
return res.status(400).json({ error: 'binId belongs to a different project' });
|
||||
}
|
||||
}
|
||||
|
||||
const assetId = uuidv4();
|
||||
const s3Key = `originals/${assetId}/${filename}`;
|
||||
const mimeType = contentType || req.file.mimetype;
|
||||
|
|
|
|||
Loading…
Reference in a new issue