Fix expired live MCP publication repair links

Renew only the authorized live attachment repair expiry, preserving immutable identity and verified cleanup. Cover actual repair PUT, replay, mismatched ownership and overlapping deletion.

Carry the exact controller identity accessor and regression from upstream 026fd9e345 (#18649); original CI two-second deadline cause remains undetermined.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-11-02 14:44:30 -06:00
1 parent 56acdfdffa
commit cb8c992af3
5 files changed
+179 -3

No files matched your search

@@ -3,7 +3,7 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import net from "node:net";
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { eq, sql } from "drizzle-orm";
import * as tables from "@paperclipai/db";
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
@@ -235,6 +235,120 @@ const support = externalDatabaseUrl ? { supported: true } : await getEmbeddedPos
} finally { await p.cleanup(); }
}, 60_000);
it("renews only an expired live agent attachment's publication repair window", async () => {
const { createPublicMcpTransfers } = await import("../services/public-mcp/file-transfers.js");
const { createPublicMcpOAuth } = await import("../services/public-mcp/oauth.js");
const { instanceSettingsService } = await import("../services/instance-settings.js");
const { default: express } = await import("express");
const { default: request } = await import("supertest");
const f = await seed(), p = await connectedMcp(f);
try {
const board = await p.oauth.authenticate(p.token), sessionId = randomUUID();
const resource = `${p.oauth.config.origin}/mcp/runner`;
await instanceSettingsService(db).updateExperimental({ enableOpenAiDot: true });
await db.update(tables.mcpOauthGrants).set({ purpose: "agent", agentId: f.agentId,
resource, scopes: ["paperclip:agent", "offline_access"] }).where(eq(tables.mcpOauthGrants.id, p.grantId));
await db.update(tables.issues).set({ status: "in_progress" }).where(eq(tables.issues.id, f.issueId));
await db.update(tables.heartbeatRuns).set({ status: "running", finishedAt: null, runtimeMode: "native",
driverKind: "grok_bot_mcp", nativeIssueId: f.issueId }).where(eq(tables.heartbeatRuns.id, f.runId));
const oauth = createPublicMcpOAuth(db, { ...p.oauth.config, resource });
const principal = await oauth.authenticate(p.token);
let ownerAvailable = true;
let ownerRunId = f.runId;
let pauseRepair = false, repairLocked!: () => void, releaseRepair!: () => void;
const locked = new Promise<void>(resolve => { repairLocked = resolve; });
const released = new Promise<void>(resolve => { releaseRepair = resolve; });
// Named work-session boundary; OAuth, transfer HTTP, storage, attachment
// registration and native publication receipt all use their real owners.
const transfers = createPublicMcpTransfers(db, oauth, p.api, p.storage, { agentSession: {
authorize: async (actor, session, taskId, _write, queryDb) => {
if (!ownerAvailable || actor.grant.id !== p.grantId || actor.grant.agentId !== f.agentId
|| session !== sessionId || taskId !== f.issueId) throw new Error("work session unavailable");
if (pauseRepair && queryDb) { pauseRepair = false; repairLocked(); await released; }
return { agentId: f.agentId, runId: ownerRunId };
},
} });
const app = express(); app.use(transfers.router);
const bytes = Buffer.from("expired live publication repair");
const args = { companyId: f.companyId, taskId: f.issueId, requestId: randomUUID(), filename: "repair.txt",
contentType: "text/plain", byteSize: bytes.length, sha256: createHash("sha256").update(bytes).digest("hex") };
const upload = await transfers.getUploadUrl(principal, args, sessionId);
const put = async (value: unknown) => {
const url = new URL(String(value));
return request(app).put(url.pathname + url.search).set("Content-Type", "text/plain").send(bytes)
.timeout({ response: 5000, deadline: 10_000 });
};
const initial = await put(upload.url);
expect(initial.status).toBe(200);
const uploadId = String(upload.uploadId), attachmentId = initial.body.attachment.id;
const current = async () => (await db.select().from(tables.mcpAttachmentUploads)
.where(eq(tables.mcpAttachmentUploads.id, uploadId)))[0];
const loseReceipt = async () => {
const [run] = await db.select().from(tables.heartbeatRuns).where(eq(tables.heartbeatRuns.id, f.runId));
const receipts = { ...run.resultJson?.semanticToolReceipts as Record<string, unknown> };
delete receipts[`external-upload:${uploadId}`];
await db.update(tables.heartbeatRuns).set({ resultJson: { ...run.resultJson, semanticToolReceipts: receipts } })
.where(eq(tables.heartbeatRuns.id, f.runId));
await db.update(tables.mcpAttachmentUploads).set({ expiresAt: new Date(0) })
.where(eq(tables.mcpAttachmentUploads.id, uploadId));
};
await loseReceipt();
const expired = await current();
expect((await put(upload.url)).status).toBe(403);
const repair = await transfers.getUploadUrl(principal, args, sessionId);
const renewed = await current();
expect(renewed).toEqual({ ...expired, expiresAt: expect.any(Date) });
expect(renewed.expiresAt.getTime()).toBeGreaterThan(Date.now());
const repaired = await put(repair.url);
expect(repaired.status).toBe(200);
expect(repaired.body).toMatchObject({ attachment: { id: attachmentId }, publication: {
source: "authenticated_upload", evidenceRef: `deliverable:${attachmentId}` } });
expect(await transfers.getUploadUrl(principal, args, sessionId)).toMatchObject({ status: "completed",
attachment: { id: attachmentId }, publication: repaired.body.publication });
expect(await current()).toEqual(renewed);
expect(await db.select().from(tables.issueAttachments).where(eq(tables.issueAttachments.issueId, f.issueId))).toHaveLength(1);
await loseReceipt();
const beforeDenied = await current();
ownerAvailable = false;
await expect(transfers.getUploadUrl(principal, args, sessionId)).rejects.toThrow("work session unavailable");
expect(await current()).toEqual(beforeDenied);
ownerAvailable = true;
ownerRunId = randomUUID();
await expect(transfers.getUploadUrl(principal, args, sessionId)).rejects.toThrow();
expect(await current()).toEqual(beforeDenied);
ownerRunId = f.runId;
await db.update(tables.mcpAttachmentUploads).set({ cleanedAt: new Date(), cleanupVerifiedAt: new Date() })
.where(eq(tables.mcpAttachmentUploads.id, uploadId));
const contradictory = await current();
await expect(transfers.getUploadUrl(principal, args, sessionId)).rejects.toThrow();
expect(await current()).toEqual(contradictory);
expect((await put(repair.url)).status).toBe(403);
await db.update(tables.mcpAttachmentUploads).set({ cleanedAt: null, cleanupVerifiedAt: null })
.where(eq(tables.mcpAttachmentUploads.id, uploadId));
// Deletion owns metadata/cleanup settlement. Overlap it with the locked
// repair to prove renewal cannot revive a pointer or recreate its bytes.
pauseRepair = true;
const repairing = transfers.getUploadUrl(principal, args, sessionId);
await locked;
const deleting = p.api(board, "DELETE", `/attachments/${attachmentId}`);
let overlappingRepair: Awaited<typeof repairing>;
try {
await vi.waitFor(async () => expect(await p.storage.headObject(f.companyId, renewed.objectKey)).toEqual({ exists: false }),
{ timeout: 5000, interval: 20 });
} finally {
releaseRepair();
[overlappingRepair] = await Promise.all([repairing, deleting]);
}
const deleted = await current();
expect(deleted).toMatchObject({ attachmentId, cleanedAt: expect.any(Date), cleanupVerifiedAt: expect.any(Date) });
await expect(transfers.getUploadUrl(principal, args, sessionId)).rejects.toThrow();
expect(await current()).toEqual(deleted);
expect((await put(repair.url)).status).toBe(403);
expect((await put(overlappingRepair.url)).status).toBe(403);
expect(await p.storage.headObject(f.companyId, deleted.objectKey)).toEqual({ exists: false });
} finally { await p.cleanup(); }
}, 60_000);
it.each(["delete_failed", "object_survived", "head_failed"] as const)("retains MCP cleanup uncertainty after actual attachment removal: %s", async failure => {
const { createPublicMcpTransfers } = await import("../services/public-mcp/file-transfers.js");
const { default: express } = await import("express");
+5
View File
@@ -172,6 +172,11 @@ const GIT_INFO_CACHE_TTL_MS = 3000;
const processStartedAt = new Date().toISOString();
let gitInfoCache: { value: ServerGitInfo; expiresAt: number } | null = null;
// Lifecycle identity must not run the synchronous Git probes used by diagnostics.
export function getServerProcessStartedAt(): string {
return processStartedAt;
}
export function getServerInfoSnapshot(
opts: {
now?: number;
@@ -0,0 +1,37 @@
import { afterEach, expect, it, vi } from "vitest";
import * as childProcess from "node:child_process";
import * as hotRestart from "../hot-restart.js";
import { getServerInfoSnapshot, resetServerInfoCacheForTests } from "../../server-info.js";
import { currentNativeControllerIdentity } from "./native-restart-recovery.js";
vi.mock("node:child_process", async importOriginal => ({
...await importOriginal<typeof import("node:child_process")>(),
execFileSync: vi.fn(() => {
throw new Error("Git metadata is unavailable; identity must still work");
}),
}));
afterEach(() => {
vi.restoreAllMocks();
resetServerInfoCacheForTests();
});
it.each([true, false])("keeps controller identity independent of Git commands (OS start available=%s)", async available => {
const observed = "2026-11-02T12:00:00.000Z";
const fallback = getServerInfoSnapshot({ gitCommand: () => { throw new Error("no checkout"); } }).processStartedAt;
resetServerInfoCacheForTests();
vi.spyOn(hotRestart, "readProcessStartedAt").mockImplementation(async () => {
if (!available) throw new Error("process start unavailable");
return observed;
});
const subprocess = vi.mocked(childProcess.execFileSync);
subprocess.mockClear();
const first = await currentNativeControllerIdentity();
const second = await currentNativeControllerIdentity();
expect(first).toEqual(second);
expect(first.pid).toBe(process.pid);
expect(first.processStartedAt.toISOString()).toBe(available ? observed : fallback);
expect(subprocess).not.toHaveBeenCalled();
});
@@ -31,7 +31,7 @@ import {
} from "@paperclipai/db";
import { REMOTE_CONTROLLER_LEASE_MS } from "./native-runner-recovery-contract.js";
import { readProcessStartedAt } from "../hot-restart.js";
import { getServerInfoSnapshot } from "../../server-info.js";
import { getServerProcessStartedAt } from "../../server-info.js";
import { redactSensitiveText } from "../../redaction.js";
import { issueRecoveryActionService } from "../issue-recovery-actions.js";
import { isNewTerminalRunFailure, reportRunFailure } from "../run-failure-report.js";
@@ -157,7 +157,7 @@ async function observedProcessStart(pid: number): Promise<Date | null> {
export async function currentNativeControllerIdentity(): Promise<NativeControllerIdentity> {
const observed = await observedProcessStart(process.pid);
const fallback = new Date(getServerInfoSnapshot().processStartedAt);
const fallback = new Date(getServerProcessStartedAt());
return {
bootId: controllerBootId,
pid: process.pid,
@@ -228,6 +228,26 @@ export function createPublicMcpTransfers(db: Db, oauth: PublicMcpOAuth, api: Api
...(publication ? { publication } : {}) };
// An older upload can lack publication proof. Verify the same bytes
// through another bounded PUT; never manufacture a receipt from metadata.
await db.transaction(async tx => {
const [current] = await tx.select().from(mcpAttachmentUploads)
.where(eq(mcpAttachmentUploads.id, upload.id)).for("update");
if (!current || current.companyId !== p.grant.companyId || current.userId !== p.grant.userId
|| current.argumentsHash !== hash || current.workSessionId !== sessionId
|| current.grantId && current.grantId !== p.grant.id
|| current.attachmentId !== attachment.id || current.storageProvider !== storage.provider
|| current.cleanedAt || current.cleanupVerifiedAt) throw unavailable();
const owner = await workSession(p, sessionId, current.taskId, true, tx as unknown as Db);
const live = await attachmentAccess(p, attachment.id, tx as unknown as Db, sessionId);
if (!owner || live.issueId !== current.taskId || live.provider !== current.storageProvider
|| live.objectKey !== current.objectKey || live.originalFilename !== current.originalFilename
|| live.byteSize !== current.byteSize || live.sha256 !== current.sha256
|| live.originatingRunId !== owner.runId || live.createdByAgentId !== owner.agentId) throw unavailable();
// Keep the committed attachment and cleanup identity immutable. A
// concurrent deletion still removes that attachment/stamps this row;
// PUT rechecks both and never stores bytes for a committed upload.
await tx.update(mcpAttachmentUploads).set({ expiresAt: new Date(Date.now() + lifetime) })
.where(eq(mcpAttachmentUploads.id, current.id));
});
}
return { ...await ticket(p, { uploadId: upload.id }, sessionId), method: "PUT", headers: { "Content-Type": a.contentType }, uploadId: upload.id, maxBytes: MAX_ATTACHMENT_BYTES,
instructions: "Send the exact file bytes with the given Content-Type. Success saves the attachment and returns its ID. An agent upload also returns a verified publication receipt; cite its evidenceRef in your completion report. There is no separate upload completion tool. Do not share this temporary file credential. Reuse requestId after a lost response." };