diff --git a/changelog.d/610.fixed.md b/changelog.d/610.fixed.md new file mode 100644 index 00000000..ae106278 --- /dev/null +++ b/changelog.d/610.fixed.md @@ -0,0 +1 @@ +Project memory now reads each historical revision under that revision's own audience, so a role-scoped revision is not listed or rendered to a reader outside it even after the current entry moves to a wider audience. A write aimed at an entry outside the actor's audience is refused as if the entry were absent, leaving the current body and revision untouched. diff --git a/docs/architecture.md b/docs/architecture.md index c5e978d7..74c6a091 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1923,12 +1923,17 @@ never updated in place). `src/lib/memory.ts` holds the rules that are not SQL. passes. An identical write is a no-op and creates no revision. - **Reads are exact.** A read names a revision and returns that body, its `sha256` digest and whether it is historical. Nothing is served from a cache - and no read returns "the latest" when a revision was asked for. + and no read returns "the latest" when a revision was asked for. A revision is + read under its own audience, never the current entry's, so a historical + revision written for another role is neither listed nor rendered after the + entry moves to a wider audience. - **Audience is a role, not a permission.** An entry is `project` or `role:`. The reader's role is the repository's own chosen role, so an administrator acting as a developer reads the developer view, and an entry outside the audience is reported as absent (404) rather than listed. - A writer may not author an entry for a role it cannot act as. + A writer may not author an entry for a role it cannot act as, and a write aimed + at an entry outside the actor's audience is refused as absent before the + revision fence, leaving the row untouched. - **The page is reachable.** The repository page's memory card links to `/repos/:id/memory`; the page is not an orphan that only a typed URL opens. - **Sources are shown.** An exact-revision read lists that revision's `sources` diff --git a/docs/security.md b/docs/security.md index 1c99acfc..24b3264b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -723,6 +723,11 @@ shared with the project or scoped to one role (`role:maintainer`, `role:developer`). The audience is resolved from the repository's chosen role, not from the account's permission: an administrator acting as a developer reads the developer view and an entry outside that audience is reported as absent, so -a role boundary is not a list filter that a direct URL walks around. A writer may -not author an entry for a role it cannot act as, and the store refuses a write -whose `expected_revision` is stale instead of overwriting the current body. +a role boundary is not a list filter that a direct URL walks around. A revision +is read under its own audience, so an old revision written for another role is +neither listed nor rendered even after the current entry moves to a wider one. +A writer may not author an entry for a role it cannot act as, and a write aimed +at an entry outside the actor's audience is refused as if the entry were absent, +before the revision fence, so a hidden key is not a write target or a revision +oracle. The store refuses a write whose `expected_revision` is stale instead of +overwriting the current body. diff --git a/src/app.ts b/src/app.ts index 232ab144..28541164 100644 --- a/src/app.ts +++ b/src/app.ts @@ -102,7 +102,7 @@ import { withDensity } from './views/layout.js' import { gatePage } from './views/gate.js' import { planHash, validateGatePlan } from './gates/plan.js' import { memoryMissingPage, memoryPage } from './views/memory.js' -import { memoryDigest, parseMemoryInput, searchMemories, visibleMemories } from './lib/memory.js' +import { audienceVisible, memoryDigest, parseMemoryInput, searchMemories, visibleMemories } from './lib/memory.js' import { DENSITY_COOKIE, isDensity, resolveDensity } from './views/density.js' import { THEME_COOKIE, isTheme, resolveTheme, withTheme } from './views/theme.js' import { GithubClient, parseRepoInput, prepareRepo, prepareRepoWithInstallation, repoPermissionWithToken } from './repos/github.js' @@ -1937,11 +1937,14 @@ export async function buildApp(options: BuildOptions): Promise { if (entry === undefined) { return htmlPage(reply, 404, memoryMissingPage({ config, locale, viewer: viewOf(viewer), repo })) } - versions = store.listMemoryVersions(entry.id) + // A revision carries its own audience: the entry may have moved to a + // wider audience, but an old revision written for another role stays + // outside this reader's audience and is neither listed nor rendered. + versions = store.listMemoryVersions(entry.id).filter((row) => audienceVisible(row.audience, role)) const rawRevision = queryText(request, 'rev') const wanted = rawRevision === undefined ? entry.revision : Number(rawRevision) const version = Number.isInteger(wanted) ? store.getMemoryVersion(entry.id, wanted) : undefined - if (version === undefined) { + if (version === undefined || !audienceVisible(version.audience, role)) { return htmlPage(reply, 404, memoryMissingPage({ config, locale, viewer: viewOf(viewer), repo })) } detail = { entry, version, historical: version.revision !== entry.revision } @@ -2068,6 +2071,13 @@ export async function buildApp(options: BuildOptions): Promise { return reply.redirect(`/repos/${repo.id}/memory?refused=${encodeURIComponent(parsed.refused)}`) } const input = parsed.value + // An entry outside the actor's audience is not a write target. Refuse it + // before the revision fence so a hidden key is not a revision oracle, and use + // the same refusal an update to a missing entry gives. + const current = store.getMemory(repo.id, input.key) + if (current !== undefined && !audienceVisible(current.audience, role)) { + return reply.redirect(`/repos/${repo.id}/memory?refused=memory_not_found`) + } const result = store.writeMemory({ repoId: repo.id, key: input.key, diff --git a/test/projectMemory.test.ts b/test/projectMemory.test.ts index 2c5a79de..eabf2889 100644 --- a/test/projectMemory.test.ts +++ b/test/projectMemory.test.ts @@ -184,6 +184,77 @@ test('a role entry is invisible to the other role, even to an administrator acti } }) +test('a revision is read under its own audience, not the current entry audience', async () => { + const h = await harness({ repoRole: 'maintainer' }) + try { + await write(h, entry({ key: 'deploy-window', audience: 'role:maintainer', body: 'Maintainer-only deploy window.' })) + const second = await write(h, entry({ + key: 'deploy-window', + audience: 'project', + body: 'Shared deploy window.', + expected_revision: '1', + })) + assert.equal(second.statusCode, 302) + assert.match(location(second), /saved=1/) + + // The current revision moved to `project`, so a developer reads the entry, + // but revision 1 was written for maintainers and stays outside that audience. + h.store.updateRepo(h.repo.id, { repo_role: 'developer' }) + const current = await page(h, '?key=deploy-window') + assert.equal(current.statusCode, 200) + assert.match(current.body, /Shared deploy window\./) + assert.match(current.body, /data-memory-version-count="1"/, 'only visible revisions are counted') + assert.doesNotMatch(current.body, /data-memory-version="1"/, 'the hidden revision is not listed') + + const hidden = await page(h, '?key=deploy-window&rev=1') + assert.equal(hidden.statusCode, 404, 'a hidden revision is not rendered through a visible entry') + assert.doesNotMatch(hidden.body, /Maintainer-only deploy window\./) + } finally { + await h.close() + } +}) + +test('a write aimed at a hidden entry is refused and leaves the row untouched', async () => { + const h = await harness({ repoRole: 'maintainer' }) + try { + await write(h, entry({ key: 'deploy-window', audience: 'role:maintainer', body: 'Maintainer-only deploy window.' })) + const before = h.store.getMemory(h.repo.id, 'deploy-window') + assert.ok(before !== undefined) + + h.store.updateRepo(h.repo.id, { repo_role: 'developer' }) + const attempt = await write(h, entry({ + key: 'deploy-window', + audience: 'project', + body: 'Developer overwrite.', + expected_revision: '1', + })) + assert.equal(attempt.statusCode, 302) + assert.match(location(attempt), /refused=memory_not_found/) + assert.doesNotMatch(location(attempt), /saved=1|conflict=/, 'the refusal neither writes nor names the revision') + + const after = h.store.getMemory(h.repo.id, 'deploy-window') + assert.equal(after?.body, 'Maintainer-only deploy window.') + assert.equal(after?.revision, 1) + assert.equal(after?.audience, 'role:maintainer') + assert.equal(h.store.listMemoryVersions(before.id).length, 1, 'the refused write appended no revision') + + // The guard runs before the revision fence: a deliberately wrong revision + // still gets the same refusal, so a hidden target is not a revision oracle. + const wrongRevision = await write(h, entry({ + key: 'deploy-window', + audience: 'project', + body: 'Developer overwrite.', + expected_revision: '99', + })) + assert.equal(wrongRevision.statusCode, 302) + assert.match(location(wrongRevision), /refused=memory_not_found/) + assert.doesNotMatch(location(wrongRevision), /conflict=/, 'a hidden target is not a revision oracle') + assert.equal(h.store.getMemory(h.repo.id, 'deploy-window')?.body, 'Maintainer-only deploy window.') + } finally { + await h.close() + } +}) + test('reading an old revision returns the old body and marks it historical', async () => { const h = await harness() try {