fix: close issue privacy revocation gaps

Authorize privacy mutations against the subtree root, avoid caching positive grants across revocation, and preserve uniform 404 read denials.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-08-01 22:54:01 +00:00
1 parent 3c7b4e6ee6
commit 15c7eff282
4 files changed
+37 -9

No files matched your search

@@ -2207,7 +2207,7 @@ describeEmbeddedPostgres("authorization service", () => {
const previousMode = process.env.PAPERCLIP_ISSUE_PRIVACY_MODE;
const previousTtl = process.env.PAPERCLIP_ISSUE_PRIVACY_CACHE_TTL_MS;
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
process.env.PAPERCLIP_ISSUE_PRIVACY_CACHE_TTL_MS = "0";
process.env.PAPERCLIP_ISSUE_PRIVACY_CACHE_TTL_MS = "5000";
try {
const company = await createCompany(db, "IssuePrivacy");
const ownerId = await createUser(db);
@@ -218,11 +218,13 @@ describe("external object routes", () => {
const app = await createApp(ownerActor());
const summary = await request(app).get(`/api/issues/${issueId}/external-object-summary`);
expect(summary.status).toBe(403);
expect(summary.status).toBe(404);
expect(summary.body.error).toBe("Issue not found");
expect(mockExternalObjectsService.getIssueSummary).not.toHaveBeenCalled();
const list = await request(app).get(`/api/issues/${issueId}/external-objects`);
expect(list.status).toBe(403);
expect(list.status).toBe(404);
expect(list.body.error).toBe("Issue not found");
expect(mockExternalObjectsService.listForIssue).not.toHaveBeenCalled();
});
+26 -5
View File
@@ -3522,6 +3522,25 @@ export function issueRoutes(
return false;
}
async function resolveIssuePrivacyManagementRoot(
req: Request,
res: Response,
issue: Parameters<typeof decideIssueAccess>[1] & {
privacyRootIssueId: string | null;
responsibleUserId: string | null;
createdByUserId: string | null;
},
) {
const privacyRootId = issue.privacyRootIssueId ?? issue.id;
const privacyRoot = privacyRootId === issue.id
? issue
: await getAccessibleResource(req, res, svc.getById(privacyRootId), "Issue not found");
if (!privacyRoot) return null;
if (privacyRoot.id !== issue.id && !(await assertIssueReadAllowed(req, res, privacyRoot))) return null;
if (!(await assertCanManageIssuePrivacy(req, res, privacyRoot))) return null;
return privacyRoot;
}
async function assertAgentIssueCommentAllowed(
req: Request,
res: Response,
@@ -7834,8 +7853,9 @@ export function issueRoutes(
router.get("/issues/:id/access-grants", async (req, res) => {
const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found");
if (!issue || !(await assertIssueReadAllowed(req, res, issue))) return;
if (!(await assertCanManageIssuePrivacy(req, res, issue))) return;
const grantIssueIds = [...new Set([issue.id, issue.privacyRootIssueId ?? issue.id])];
const privacyRoot = await resolveIssuePrivacyManagementRoot(req, res, issue);
if (!privacyRoot) return;
const grantIssueIds = [...new Set([issue.id, privacyRoot.id])];
const grants = await db
.select()
.from(issueAccessGrants)
@@ -7850,8 +7870,9 @@ export function issueRoutes(
async (req, res) => {
const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found");
if (!issue || !(await assertIssueReadAllowed(req, res, issue))) return;
if (!(await assertCanManageIssuePrivacy(req, res, issue))) return;
const grantIssueIds = [...new Set([issue.id, issue.privacyRootIssueId ?? issue.id])];
const privacyRoot = await resolveIssuePrivacyManagementRoot(req, res, issue);
if (!privacyRoot) return;
const grantIssueIds = [...new Set([issue.id, privacyRoot.id])];
const [grant] = await db
.update(issueAccessGrants)
.set({ revokedAt: new Date() })
@@ -7886,7 +7907,7 @@ export function issueRoutes(
const id = req.params.id as string;
const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
if (!existing) return;
if (req.body.visibility !== undefined && !(await assertCanManageIssuePrivacy(req, res, existing))) return;
if (req.body.visibility !== undefined && !(await resolveIssuePrivacyManagementRoot(req, res, existing))) return;
assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body));
if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return;
if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, existing, req.body))) return;
+6 -1
View File
@@ -520,7 +520,12 @@ async function actorHasIssuePrivacyGrant(db: Db, actor: AuthorizationActor, issu
.limit(1)
.then((rows) => rows.length > 0);
issuePrivacyGrantCache.set(cacheKey, { expiresAt: now + issuePrivacyCacheTtlMs(), promise });
return promise;
const granted = await promise;
// Positive authorization decisions must observe revocation on the next read.
// Keep only negative decisions cached; grant creation paths already tolerate
// their short TTL, while revocation must fail closed immediately.
if (granted) issuePrivacyGrantCache.delete(cacheKey);
return granted;
}
/** Canonical row-level issue privacy predicate, also consumed by run-derived checks. */