fix: read and write project memory under the target revision's audience (#610)

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.

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:<maintainer|developer>`. 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`

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.

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<BuiltApp> {
       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<BuiltApp> {
       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,

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 {