diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e8d649..e547d706 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ All notable changes to stellar-api are documented here. - `GET /communities/{id}` now declares its `403` in the contract. - **A report opens the release it concerns, for `reports_manage`** (#905, ADR-0055 §3). - While a report is `Open` or `Claimed`, a `reports_manage` holder reads `GET /communities/{id}/releases/{rid}` and `…/contributions` for the release it targets. A report concerns a release when it targets the release, one of its contributions, or a comment in either's thread. Before, a closed community they held no role in answered `403`. - - **Read only.** That read's contributions carry `downloadUrl` as an empty string. Downloads, edits, votes, other releases and every list stay member-only. Resolving the report closes the page again. + - **Read only.** That read's contributions carry no `downloadUrl`, as for every reader since #908. Downloads, edits, votes, other releases and every list stay member-only. Resolving the report closes the page again. - **The staff queue's `sourceUrl` follows the same rule.** It linked every release before, including ones whose page refused staff. A link into a release page now appears only when the page will open, so a resolved report in a closed community links nowhere. - **`npm run e2e:korin`** (`src/scripts/seed-korin-e2e.ts`): fixtures for the korin end-to-end run of private-community announce delivery (#328). It seeds a cast and three communities, then adds contributions, removes members and flips communities to PRIVATE on demand. It refuses any database not named `stellar_e2e`. @@ -43,6 +43,11 @@ All notable changes to stellar-api are documented here. - **Removed** `parsedBody`, `parsedQuery`, `parsedParams` and `parsedPage`, the `res.locals.parsed*` keys they read, and the `validate:handles` gate (#915). With the helpers gone and the three `res.locals` keys typed `never`, `tsc` enforces the rule. - No response or status changes. +- **A contribution's `downloadUrl` reaches a member only through the Download Grant** (#908). `GET /communities/{id}/releases/{releaseId}/contributions` sent every reader each contribution's URL, so a direct API caller could skip `POST /contributions/{id}/access` and its debit. That list no longer carries the field, for any reader. + - **Contract:** `ReleaseContributionDetail`, `ReleaseContribution` (the release detail's `contributions[]`) and `Contribution` no longer declare `downloadUrl`. The last two declared it as required, but their reads never sent it. `Contribution` serves `GET /contributions/{id}`, `POST /contributions` and the release attach. + - The uploader's own `GET /contributions` keeps the field, declared as the new `OwnContribution`. + - A source-scan spec, `contributionDownloadUrl.spec.ts`, fails on any new query that comes back holding the URL. + ## [0.10.2] — 2026-10-02 ### Changed diff --git a/CONTEXT.md b/CONTEXT.md index 74f4a18f..1ac48e2e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -43,6 +43,10 @@ _Avoid_: contribution credit, ratio bonus, approved bytes The generic `Contribution` model — the type-agnostic unit of shared content (a Download URL) carried across every CommunityType. Holds only fields every Contribution has (ids, `downloadUrl`, `sizeInBytes`, `linkStatus`, the `type` format discriminator, accounting). A Release is the primary Contribution type; Film/eLearning/ApiPlugin follow. Type-specific metadata lives in satellite models, never on the spine (ADR-0008). _Avoid_: contribution table, base contribution, release row +**Download Grant**: +The debit-bearing act that reveals a Contribution's Download URL: `POST /contributions/:id/access` checks `canDownload`, debits the consumer's `consumed`, credits the contributor, and returns the `downloadUrl` (`downloads.ts`). No read reveals the URL without one (#908). The one exception is the uploader's own uploads list, `GET /contributions`. `/access/latest` re-serves a grant made in the last two minutes, so a retried click is not charged twice. Staff reverse a grant with `POST /downloads/:grantId/reverse`. +_Avoid_: download link, access grant, download token + **Release File**: The per-file rip-metadata satellite (`ReleaseFile`, 1:1 with a music Contribution): `bitrate`, `hasLog`, `hasCue`, `isScene` — the fingerprint the quality grade reads. Per-file, so distinct from the per-pressing `Edition`. The music analog of the satellite each future Contribution type attaches. Since #129 it is also client-surfaced — nested on the release-scoped contributions read (`GET /communities/:id/releases/:id/contributions`) that feeds the UI edition stack — not only read by the grade. _Avoid_: contribution metadata, file info, rip record diff --git a/docs/adr/0055-staff-read-the-administrative-record.md b/docs/adr/0055-staff-read-the-administrative-record.md index 5d6c1fa4..bcc36fbd 100644 --- a/docs/adr/0055-staff-read-the-administrative-record.md +++ b/docs/adr/0055-staff-read-the-administrative-record.md @@ -36,6 +36,8 @@ A holder of `reports_manage` reads **the page of the release a report concerns** That read gets each contribution's `downloadUrl` as an empty string. The URL is the download without the grant's debit, and the grant here is to look. +> **Amended 2026-10-03 ([#908](https://github.com/orphic-inc/stellar-api/issues/908)).** That read no longer carries `downloadUrl` at all. #908 removed it from the contributions list for every reader, so there is nothing left for this read to blank. + Nothing else opens: no download, no other release, and no search, feed, notification or Top 10 entry. Resolving the report ends the grant. Editing the release still goes through the workbench's own gate. Comments on requests and on communities are left out until a report needs them. A report is a request for staff to look at one thing, so it is the narrowest grant that lets them do it. It is built in [#905](https://github.com/orphic-inc/stellar-api/issues/905). diff --git a/openapi.json b/openapi.json index 2950b7fe..bdba379d 100644 --- a/openapi.json +++ b/openapi.json @@ -4418,9 +4418,6 @@ "txt" ] }, - "downloadUrl": { - "type": "string" - }, "sizeInBytes": { "type": "number", "nullable": true @@ -4455,7 +4452,6 @@ "id", "user", "type", - "downloadUrl", "collaborators" ] }, @@ -4528,9 +4524,6 @@ "txt" ] }, - "downloadUrl": { - "type": "string" - }, "sizeInBytes": { "type": "number", "nullable": true @@ -4587,12 +4580,29 @@ "user", "release", "type", - "downloadUrl", "linkStatus", "ratioExempt", "collaborators" ] }, + "OwnContribution": { + "allOf": [ + { + "$ref": "#/components/schemas/Contribution" + }, + { + "type": "object", + "properties": { + "downloadUrl": { + "type": "string" + } + }, + "required": [ + "downloadUrl" + ] + } + ] + }, "ReleaseFileQuality": { "type": "object", "properties": { @@ -4705,9 +4715,6 @@ "type": "string", "nullable": true }, - "downloadUrl": { - "type": "string" - }, "sizeInBytes": { "type": "number", "nullable": true @@ -4821,7 +4828,6 @@ "userId", "releaseId", "contributorId", - "downloadUrl", "sizeInBytes", "linkStatus", "linkCheckedAt", @@ -26431,7 +26437,7 @@ "tags": [ "Communities" ], - "description": "Readable by whoever may read the release detail, including a `reports_manage` holder whom an open report lets in (ADR-0055 §3, #905). That read gets every `downloadUrl` as an empty string: it grants a look, not a download.", + "description": "Readable by whoever may read the release detail, including a `reports_manage` holder whom an open report lets in (ADR-0055 §3, #905). No reader gets a `downloadUrl` here (#908): the download grant hands it out.", "parameters": [ { "schema": { @@ -26706,6 +26712,7 @@ "tags": [ "Contributions" ], + "description": "The caller's own uploads, the one read besides the download grant that carries each `downloadUrl` (#908).", "responses": { "200": { "description": "Paginated contributions", @@ -26717,7 +26724,7 @@ "data": { "type": "array", "items": { - "$ref": "#/components/schemas/Contribution" + "$ref": "#/components/schemas/OwnContribution" } }, "meta": { diff --git a/src/contributionDownloadUrl.spec.ts b/src/contributionDownloadUrl.spec.ts new file mode 100644 index 00000000..dd5e5b00 --- /dev/null +++ b/src/contributionDownloadUrl.spec.ts @@ -0,0 +1,253 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import ts from 'typescript'; + +/** + * No read reveals a Contribution's `downloadUrl` without a Download Grant + * (#908). The URL is the download without the grant's debit, so only the grant + * and the uploader's own uploads list may carry it. + * + * The leak #908 closed was one `downloadUrl: true` in a release read's select. + * This spec parses the source and counts, per file, the three ways a query + * comes back holding the URL: + * + * - `select` — a `downloadUrl: true` property + * - `bare` — a `.contribution.(...)` with no inline `select`, + * which returns every scalar + * - `include` — `contribution: true` or `contributions: true` inside an + * `include`, which does the same through a relation + * + * Each count must match ALLOWED exactly, and each entry says why its rows never + * reach a reader without a grant. A new site fails, and so does a second site + * in an allowed file. Remove an entry when its site goes. + * + * WHAT IT CANNOT SEE: a select built elsewhere and passed by name, a delegate + * reached through an alias, or a computed property name. Nothing in the tree + * does that for a contribution today. + */ + +type Kind = 'select' | 'bare' | 'include'; + +const ALLOWED: Record> & { why: string }> = + { + 'src/modules/downloads.ts': { + select: 1, + why: 'the Download Grant itself: it debits, then returns the URL' + }, + 'src/routes/api/downloads.ts': { + select: 1, + why: '`/access/latest`: re-serves a grant made in the last two minutes' + }, + 'src/routes/api/communities/contributions.ts': { + select: 1, + why: '`GET /contributions`: the caller’s own uploads only' + }, + 'src/modules/linkHealth.ts': { + select: 1, + why: 'the link checker probes the URL server-side and returns only a status' + }, + 'src/modules/releaseWorkbench/contributions.ts': { + bare: 1, + why: '`assertMayAttach`: an existence check, whose row is never returned' + }, + 'src/modules/requestLifecycle.ts': { + bare: 1, + why: '`fillRequest`: an ownership check, whose row is never returned' + }, + 'src/modules/devTools/generators/contributions.ts': { + bare: 1, + why: 'dev-only content factory: writes generated rows' + }, + 'src/scripts/seed-korin-e2e.ts': { + bare: 1, + why: 'e2e seed script: writes a fixture row, run from the command line' + } + }; + +const ROOTS = ['src', 'prisma']; +const SKIPPED_DIRS = new Set(['integration', 'test', 'node_modules']); +const CONTRIBUTION_METHODS = new Set([ + 'findFirst', + 'findFirstOrThrow', + 'findUnique', + 'findUniqueOrThrow', + 'findMany', + 'create', + 'createManyAndReturn', + 'update', + 'updateManyAndReturn', + 'upsert', + 'delete' +]); + +const collectFiles = (dir: string): string[] => + readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + return SKIPPED_DIRS.has(entry.name) ? [] : collectFiles(path); + } + return entry.name.endsWith('.ts') && !entry.name.endsWith('.spec.ts') + ? [path] + : []; + }); + +const nameOf = (p: ts.ObjectLiteralElementLike): string | undefined => + p.name && (ts.isIdentifier(p.name) || ts.isStringLiteral(p.name)) + ? p.name.text + : undefined; + +const isTrue = (p: ts.PropertyAssignment): boolean => + p.initializer.kind === ts.SyntaxKind.TrueKeyword; + +/** Is `expr` the `contribution` delegate, as in `prisma.contribution`? */ +const isContributionDelegate = (expr: ts.Expression): boolean => + (ts.isPropertyAccessExpression(expr) && expr.name.text === 'contribution') || + (ts.isElementAccessExpression(expr) && + ts.isStringLiteral(expr.argumentExpression) && + expr.argumentExpression.text === 'contribution'); + +const hasInlineSelect = (arg: ts.Expression | undefined): boolean => + !!arg && + ts.isObjectLiteralExpression(arg) && + arg.properties.some((p) => nameOf(p) === 'select'); + +/** Is this property the value of an `include: { ... }`? */ +const isInsideInclude = (p: ts.PropertyAssignment): boolean => { + const obj = p.parent; + return ( + ts.isPropertyAssignment(obj.parent) && nameOf(obj.parent) === 'include' + ); +}; + +const RELATION_NAMES = new Set(['contribution', 'contributions']); + +/** A `downloadUrl: true` select, or a whole-row relation include. */ +const propertyKind = (node: ts.PropertyAssignment): Kind | null => { + if (!isTrue(node)) return null; + const name = nameOf(node); + if (name === 'downloadUrl') return 'select'; + if (name && RELATION_NAMES.has(name) && isInsideInclude(node)) { + return 'include'; + } + return null; +}; + +/** A `.contribution.(...)` with no inline `select`. */ +const isBareContributionCall = (node: ts.CallExpression): boolean => + ts.isPropertyAccessExpression(node.expression) && + isContributionDelegate(node.expression.expression) && + CONTRIBUTION_METHODS.has(node.expression.name.text) && + !hasInlineSelect(node.arguments[0]); + +const kindOf = (node: ts.Node): Kind | null => { + if (ts.isPropertyAssignment(node)) return propertyKind(node); + if (ts.isCallExpression(node) && isBareContributionCall(node)) return 'bare'; + return null; +}; + +const countSites = ( + fileName: string, + source: string +): Partial> => { + const sf = ts.createSourceFile( + fileName, + source, + ts.ScriptTarget.Latest, + true + ); + const counts: Partial> = {}; + const visit = (node: ts.Node) => { + const kind = kindOf(node); + if (kind) counts[kind] = (counts[kind] ?? 0) + 1; + ts.forEachChild(node, visit); + }; + visit(sf); + return counts; +}; + +describe('no read reveals a downloadUrl without a grant (#908)', () => { + it('finds exactly the allowed sites across src and prisma', () => { + const found: Record>> = {}; + for (const file of ROOTS.flatMap(collectFiles)) { + const counts = countSites(file, readFileSync(file, 'utf8')); + if (Object.keys(counts).length) found[file] = counts; + } + const expected = Object.fromEntries( + Object.entries(ALLOWED).map(([file, { why: _why, ...counts }]) => [ + file, + counts + ]) + ); + expect(found).toEqual(expected); + }); + + // Tried to fool it before trusting it. + describe('the checker', () => { + const check = (body: string) => countSites('fixture.ts', body); + + it('counts a downloadUrl select, nested or not', () => { + expect( + check( + 'prisma.release.findMany({ select: { contributions: { select: { downloadUrl: true } } } });' + ) + ).toEqual({ select: 1 }); + }); + + it('ignores a downloadUrl written as data', () => { + expect( + check( + 'prisma.contribution.update({ where, data: { downloadUrl: url }, select: { id: true } });' + ) + ).toEqual({}); + }); + + it('counts a contribution read with no select', () => { + expect(check('tx.contribution.findMany({ where });')).toEqual({ + bare: 1 + }); + expect(check('prisma.contribution.findUnique(args);')).toEqual({ + bare: 1 + }); + expect(check("prisma['contribution'].findFirst({});")).toEqual({ + bare: 1 + }); + }); + + it('counts an include on a contribution read as bare too', () => { + expect( + check('prisma.contribution.findFirst({ include: { release: true } });') + ).toEqual({ bare: 1 }); + }); + + it('passes a contribution read with an inline select', () => { + expect( + check('prisma.contribution.findMany({ select: { id: true } });') + ).toEqual({}); + }); + + it('counts a whole-row relation include', () => { + expect( + check('prisma.release.findMany({ include: { contributions: true } });') + ).toEqual({ include: 1 }); + expect( + check("prisma.grant.findMany({ include: { 'contribution': true } });") + ).toEqual({ include: 1 }); + }); + + it('ignores a relation count', () => { + expect( + check( + 'prisma.release.findMany({ select: { _count: { select: { contributions: true } } } });' + ) + ).toEqual({}); + }); + + it('ignores count, exists checks and other models', () => { + expect( + check( + 'prisma.contribution.count({ where }); prisma.contribution.deleteMany({ where }); prisma.release.findMany({ where });' + ) + ).toEqual({}); + }); + }); +}); diff --git a/src/integration/releaseWorkbench.integration.ts b/src/integration/releaseWorkbench.integration.ts index 6c55460d..da1f63ec 100644 --- a/src/integration/releaseWorkbench.integration.ts +++ b/src/integration/releaseWorkbench.integration.ts @@ -110,7 +110,7 @@ describe('listReleaseContributions', () => { // Spine facts — format is the contribution type, size normalized to a number. expect(entry.type).toBe(FileType.flac); expect(entry.sizeInBytes).toBe(5_000_000_000); - expect(entry.downloadUrl).toBe('https://example.com/kob.torrent'); + expect(entry).not.toHaveProperty('downloadUrl'); // Rip-quality satellite (ReleaseFile) — the whole point of #129. expect(entry.releaseFile).not.toBeNull(); @@ -128,6 +128,34 @@ describe('listReleaseContributions', () => { expect(entry.edition?.isRemaster).toBe(true); }); + // The URL is the download without the grant's debit (#908): only the grant + // and the uploader's own list carry it. + it("gives a community member no contribution's download URL", async () => { + const uploader = await createUser('uploader'); + const member = await createUser('member'); + const community = await createCommunity(); + await testPrisma.consumer.create({ + data: { + userId: member.id, + communities: { connect: { id: community.id } } + } + }); + + const created = await createContributionSubmission({ + userId: uploader.id, + input: losslessInput(community.id) + }); + + const contributions = await listReleaseContributions({ + actorId: member.id, + communityId: community.id, + releaseId: created!.releaseId + }); + + expect(contributions).toHaveLength(1); + expect(contributions[0]).not.toHaveProperty('downloadUrl'); + }); + it('returns an empty array for a release with no contributions', async () => { const user = await createUser('empty'); const community = await createCommunity(); diff --git a/src/integration/reportScopedRelease.integration.ts b/src/integration/reportScopedRelease.integration.ts index 8dabfa44..08e881ab 100644 --- a/src/integration/reportScopedRelease.integration.ts +++ b/src/integration/reportScopedRelease.integration.ts @@ -244,7 +244,7 @@ describe('a report opens the release it concerns (ADR-0055 §3)', () => { const { detail, contributions } = await readPage(staffId); expect(detail.status).toBe(200); - expect(contributions.body[0].downloadUrl).toBe(''); + expect(contributions.body[0]).not.toHaveProperty('downloadUrl'); const edit = await request(app) .put(`/api${releasePath(releaseId)}`) .set(session(staffId)) @@ -252,11 +252,12 @@ describe('a report opens the release it concerns (ADR-0055 §3)', () => { expect(edit.status).toBe(403); }); - it('gives a member the download URL as before', async () => { + // No reader gets it from this list, the uploader included (#908): their + // own uploads list carries it, and the grant hands it to everyone else. + it('gives the uploader no download URL here either', async () => { const { contributions } = await readPage(uploaderId); - expect(contributions.body[0].downloadUrl).toBe( - 'https://example.com/file.torrent' - ); + expect(contributions.status).toBe(200); + expect(contributions.body[0]).not.toHaveProperty('downloadUrl'); }); it('closes again, with its link, once the report is resolved', async () => { diff --git a/src/lib/openapi.ts b/src/lib/openapi.ts index 92095a94..28660be0 100644 --- a/src/lib/openapi.ts +++ b/src/lib/openapi.ts @@ -4625,7 +4625,6 @@ const ReleaseContribution = registry.register( username: z.string() }), type: z.nativeEnum(FileType), - downloadUrl: z.string(), sizeInBytes: z.number().nullable().optional(), collaborators: z.array( z.object({ @@ -4652,7 +4651,6 @@ const Contribution = registry.register( communityId: z.number().nullable().optional() }), type: z.nativeEnum(FileType), - downloadUrl: z.string(), sizeInBytes: z.number().nullable().optional(), linkStatus: z.enum(['UNKNOWN', 'PASS', 'WARN', 'FAIL']), linkCheckedAt: z.string().nullable().optional(), @@ -4668,6 +4666,14 @@ const Contribution = registry.register( }) ); +// The uploader's own row (#908). Only `GET /contributions` carries the +// `downloadUrl`; every other read of a contribution leaves it out, because the +// URL is the download without the grant's debit. +const OwnContribution = registry.register( + 'OwnContribution', + Contribution.extend({ downloadUrl: z.string() }) +); + // The per-file rip-quality satellite (ReleaseFile), nested on a release-scoped // contribution read. `bitrate` is null until graded. const ReleaseFileQuality = registry.register( @@ -4731,7 +4737,6 @@ const ReleaseContributionDetail = registry.register( releaseId: z.number(), contributorId: z.number(), releaseDescription: z.string().nullable().optional(), - downloadUrl: z.string(), sizeInBytes: z.number().nullable(), linkStatus: z.enum(['UNKNOWN', 'PASS', 'WARN', 'FAIL']).nullable(), linkCheckedAt: z.string().nullable(), @@ -5187,8 +5192,8 @@ registry.registerPath({ // One contribution on a community browse row (#728), read off `releaseBrowse`'s // select rather than borrowed from `ReleaseContribution`: the browse sends no -// `downloadUrl` or `collaborators`, and does send link health, the ratio -// exemption and the consumer count. +// `collaborators`, and does send link health, the ratio exemption and the +// consumer count. const ReleaseBrowseContribution = registry.register( 'ReleaseBrowseContribution', z.object({ @@ -6278,7 +6283,7 @@ registry.registerPath({ path: '/communities/{communityId}/releases/{releaseId}/contributions', tags: ['Communities'], description: - 'Readable by whoever may read the release detail, including a `reports_manage` holder whom an open report lets in (ADR-0055 §3, #905). That read gets every `downloadUrl` as an empty string: it grants a look, not a download.', + 'Readable by whoever may read the release detail, including a `reports_manage` holder whom an open report lets in (ADR-0055 §3, #905). No reader gets a `downloadUrl` here (#908): the download grant hands it out.', request: { params: z.object({ communityId: z.string(), @@ -6338,13 +6343,16 @@ registry.registerPath({ method: 'get', path: '/contributions', tags: ['Contributions'], + description: + "The caller's own uploads, the one read besides the download grant that " + + 'carries each `downloadUrl` (#908).', responses: { 200: { description: 'Paginated contributions', content: { 'application/json': { schema: z.object({ - data: z.array(Contribution), + data: z.array(OwnContribution), meta: PaginationMeta }) } diff --git a/src/modules/releaseWorkbench/contributions.ts b/src/modules/releaseWorkbench/contributions.ts index 88c1f7f0..3cdf2a9d 100644 --- a/src/modules/releaseWorkbench/contributions.ts +++ b/src/modules/releaseWorkbench/contributions.ts @@ -20,7 +20,6 @@ const releaseContributionDetailSelect = { releaseId: true, contributorId: true, releaseDescription: true, - downloadUrl: true, sizeInBytes: true, linkStatus: true, linkCheckedAt: true, @@ -50,15 +49,16 @@ const releaseContributionDetailSelect = { // The rip-quality satellite + full edition identity for one release's // contributions. Kept off the release detail view (which is growing heavy) and // served from its own release-scoped GET so the UI can lazy-load an edition -// stack (bitrate/media/flags) on demand. Gated identically to the detail read. +// stack (bitrate/media/flags) on demand. Gated identically to the detail read, +// including a read a report opens (ADR-0055 §3, #905). // -// A read a report opens (ADR-0055 §3, #905) gets every `downloadUrl` as an empty -// string: the grant is to look, and the URL is the download without the -// grant's debit. The empty string keeps the contract's shape. +// It carries no `downloadUrl`, for any viewer (#908): the URL is the download +// without the grant's debit, so only the grant and the uploader's own list +// hand it out. export const listReleaseContributions = async ( ref: ReleaseWorkbenchRef ): Promise => { - const { reportScoped } = await loadReleaseWorkbenchAuthority(ref, { + await loadReleaseWorkbenchAuthority(ref, { allowReportScoped: true }); @@ -83,7 +83,6 @@ export const listReleaseContributions = async ( return contributions.map((contribution) => ({ ...contribution, - downloadUrl: reportScoped ? '' : contribution.downloadUrl, sizeInBytes: sizeBytesToNumber(contribution.sizeInBytes) })); }; diff --git a/src/modules/releaseWorkbench/types.ts b/src/modules/releaseWorkbench/types.ts index 452e759e..6dcd0863 100644 --- a/src/modules/releaseWorkbench/types.ts +++ b/src/modules/releaseWorkbench/types.ts @@ -57,7 +57,6 @@ export type ReleaseContributionDetailView = { releaseId: number; contributorId: number; releaseDescription: string | null; - downloadUrl: string; sizeInBytes: number | null; linkStatus: string | null; linkCheckedAt: Date | null;