mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 22:56:58 +01:00
## Thinking Path > - Paperclip lets people manage agent work and approve app actions. > - An approval must bind the action that the person reviewed. > - OAuth access tokens can expire while that person is away. > - The old target snapshot treated each token refresh as a new authorization. It also imposed a one-hour review deadline. > - This pull request separates token renewal from authorization identity and keeps the human decision durable. > - Current access, the reviewed arguments, meaningful target changes, task closure, and execution receipts still control dispatch. > - People can approve later without repeating a decision after routine connection maintenance. ## Linked Issues or Issue Description **What happened?** A pending app action returned `approved_tool_target_changed` after its OAuth tokens refreshed. The tool and connection settings were unchanged. The same card also expired after one hour. A sign-in problem could discard an accepted decision before provider dispatch. **Expected behavior** The original card remains reviewable after routine OAuth refresh and a server restart. A later approval runs the exact reviewed action once. A connection repair preserves the decision while Paperclip verifies the same authorization. Meaningful changes require a new review. **Steps to reproduce** 1. Connect an OAuth MCP app and configure an action to require approval. 2. Request the action, then refresh its access and refresh tokens before approving. 3. Approve the original card. Before this change, it returns HTTP 409 even when the authorization is unchanged. 4. Wait more than one hour. Before this change, the card expires without a task-specific deadline. **Paperclip version or commit** Reproduced from master at `76254ead2229`. The final branch is rebased onto current master. ## What Changed - Add versioned approval target comparison. New snapshots bind credential identity epochs, consent, permissions, ownership, connection configuration, tool contracts, and credential references. Authenticated OAuth refresh preserves identity. Manual credential replacement does not. - Remove the implicit one-hour deadline from ordinary app action reviews. Keep explicit deadlines and terminal-task expiry. Synchronize the nullable interaction deadline in shared contracts and UI. - Keep accepted, undispatched actions when sign-in is required. Show a saved-approval repair state. Resume on committed connection changes after target and access revalidation, without an idle retry loop. - Preserve the existing execution claim, result receipt, and uncertain-outcome no-replay rules. - Explain meaningful target changes without inviting the person to retry the stale card. - Add a production task-thread Storybook example and document compatibility and recovery behavior. ## Verification - Red: production-refresher invalid-grant handling and formal-approval withdrawal during a sign-in wait also reproduced as failing regressions before their fixes. - Red: before implementation, the local OAuth HTTP fixture failed for both public and confidential clients with `approved_tool_target_changed` after token refresh. - Green: the extended fixture passes for public and confidential clients with personal and shared grants. It covers actual local HTTP token exchanges, multiple refreshes, a rebuilt gateway, an ended originating run, approval after 25 hours, duplicate approval, preserved sharing arguments, sign-in waiting, and event-driven repair. The gateways use the production OAuth refresher; an invalid refresh preserves the accepted decision before provider dispatch. Omitted-scope responses cover equivalent scope-reporting changes. The provider is an isolated local fixture, not a deployed Notebook service. - Negative controls refuse changed consent, scopes, tool definition, destination, and manual credential replacement before provider execution. - Existing gateway service, connected-tool acceptance, secret owner, secret service, unsigned-review queue, and connection-health suites passed. Explicit-deadline fixtures still prove expiry behavior, including deadlines on accepted sign-in waits. A withdrawn formal approval cancels a waiting action before connection recovery. - Approval card, task-chat, error-copy and target-comparison tests passed. Server and UI typechecks passed. Token gates and Storybook catalog checks passed. - Inspected the new saved-approval repair card in local Storybook using the production task thread. It shows a reconnect link and no second approval button or false running state. - `pnpm verify --explain` returned exit 2 and requested broad CI coverage. [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507) passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build, typecheck/release registry, general and serialized tests, and all eight browser shards. - Greptile reviewed the same final head at 5/5. Both findings from the first review are fixed and their threads resolved; no open follow-ups remain. ## Risks - No database migration is needed. The new comparison reuses the vault identity epoch. Only authenticated OAuth refresh code can preserve that epoch; HTTP credential-write schemas cannot select that behavior. - Earlier signed requests keep their original exact-version checks and saved deadlines. Already-expired cards are not revived. A historical card whose credentials already changed needs a fresh review. - A new consent, account, permission set, destination, or manually replaced credential must not inherit the old authorization. Reconnection continues automatically only when revalidation proves the same saved authorization. - The interaction enrichment now permits a null deadline. Existing string deadlines remain valid. The web client and shared validator change together. - This PR does not deploy to staging or perform a live Notebook publication. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
110 lines
5.3 KiB
TypeScript
110 lines
5.3 KiB
TypeScript
import { canonicalToolArguments } from "./tool-content-guards.js";
|
|
|
|
function record(value: unknown): Record<string, unknown> | null {
|
|
return value !== null && typeof value === "object" && !Array.isArray(value)
|
|
? value as Record<string, unknown> : null;
|
|
}
|
|
|
|
/** Refresh bookkeeping does not change the OAuth authorization being reviewed. */
|
|
export function oauthApprovalIdentity(value: unknown): Record<string, unknown> {
|
|
const { expiresAt, accessTokenExpiresAt, refreshTokenExpiresAt, refreshedAt, refreshLease,
|
|
scope, scopeSource, unrequestedScopes,
|
|
...identity } = record(value) ?? {};
|
|
for (const key of ["scopes", "requestedScopes"]) {
|
|
if (Array.isArray(identity[key])) identity[key] = [...new Set(identity[key])].sort();
|
|
}
|
|
return identity;
|
|
}
|
|
|
|
export function oauthApprovalConfig(value: Record<string, unknown>): Record<string, unknown> {
|
|
const result = { ...value };
|
|
if (record(value.oauth)) result.oauth = oauthApprovalIdentity(value.oauth);
|
|
const metadata = record(value.providerMetadata);
|
|
if (metadata && record(metadata.oauth)) {
|
|
result.providerMetadata = { ...metadata, oauth: oauthApprovalIdentity(metadata.oauth) };
|
|
}
|
|
return result;
|
|
}
|
|
|
|
export function oauthGrantApprovalIdentity(value: unknown, connectionOAuth: unknown) {
|
|
const grant = oauthApprovalIdentity(value);
|
|
const connection = record(connectionOAuth) ?? {};
|
|
const tokenType = grant.tokenType ?? connection.tokenType ?? "Bearer";
|
|
return { ...grant, strategy: grant.strategy ?? connection.strategy ?? "direct_oauth",
|
|
tokenType: typeof tokenType === "string" ? tokenType.toLowerCase() : tokenType };
|
|
}
|
|
|
|
export function approvalGrantTenantIdentity(value: unknown, connectionOAuth: unknown) {
|
|
const tenant: Record<string, unknown> = { ...record(value), oauth: oauthGrantApprovalIdentity(record(value)?.oauth, connectionOAuth) };
|
|
const github = record(tenant.github);
|
|
if (github) {
|
|
const { lastAccessRefreshAt, bindingSync, continuityRetry, lastWebhookAt, webhookHealth, ...identity } = github;
|
|
tenant.github = identity;
|
|
}
|
|
return tenant;
|
|
}
|
|
|
|
export function externalCredentialApprovalIdentity(value: unknown) {
|
|
const fields = record(value);
|
|
if (fields?.provider !== "vercel_connect") return value;
|
|
const { expiresAt, lastVerifiedAt, ...identity } = fields;
|
|
return identity;
|
|
}
|
|
|
|
function credentials(value: unknown, durable: boolean): unknown {
|
|
if (!Array.isArray(value)) return value;
|
|
return value.map(entry => {
|
|
const fields = record(entry);
|
|
if (!fields) return entry;
|
|
const { identityEpoch, resolvedVersion, ...rest } = fields;
|
|
// Only current, verified OAuth token identities can survive version changes.
|
|
// Fixed versions and non-OAuth secrets retain exact version comparisons.
|
|
return durable && Number.isSafeInteger(identityEpoch) && Number(identityEpoch) >= 0
|
|
? { ...rest, identityEpoch }
|
|
: { ...rest, resolvedVersion };
|
|
});
|
|
}
|
|
|
|
function comparable(snapshot: Record<string, unknown>, durable: boolean) {
|
|
const { approvalSnapshotVersion, connectionIdentityHash, connectionAuthorizationConfigHash,
|
|
connectionAuthorizationTransportConfigHash, credentialGrantIdentityHash, ...legacy } = snapshot;
|
|
const result = durable ? { ...snapshot } : legacy;
|
|
if (durable) {
|
|
delete result.connectionConfigHash;
|
|
delete result.connectionTransportConfigHash;
|
|
}
|
|
for (const key of ["headerCredentialVersions", "credentialSecretVersions"]) {
|
|
if (key in result) result[key] = credentials(result[key], durable);
|
|
}
|
|
return result;
|
|
}
|
|
|
|
export function approvalSnapshotsMatch(reviewed: unknown, live: Record<string, unknown> | null): boolean {
|
|
const saved = record(reviewed);
|
|
if (!saved || !live) return !saved && !live;
|
|
if (saved.approvalSnapshotVersion !== undefined && saved.approvalSnapshotVersion !== 2) return false;
|
|
// Old signed requests retain their exact-version contract. Never infer
|
|
// continuity for credentials that changed before this contract was recorded.
|
|
const durable = saved.approvalSnapshotVersion === 2;
|
|
if (durable && live.approvalSnapshotVersion !== 2) return false;
|
|
return canonicalToolArguments(comparable(saved, durable)) === canonicalToolArguments(comparable(live, durable));
|
|
}
|
|
|
|
export function approvalTargetChangeMessage(reviewed: unknown, live: Record<string, unknown> | null): string {
|
|
const saved = record(reviewed);
|
|
if (saved && live) {
|
|
const durable = saved.approvalSnapshotVersion === 2;
|
|
const before = comparable(saved, durable), after = comparable(live, durable);
|
|
const changed = (keys: string[]) => keys.some(key => canonicalToolArguments(before[key]) !== canonicalToolArguments(after[key]));
|
|
if (changed(["credentialGrantId", "credentialGrantIdentityHash", "credentialGrantRefsHash",
|
|
"credentialSecretVersions", "headerCredentialVersions"]))
|
|
return "The connected account, credentials, or permissions changed. A new review is required.";
|
|
if (changed(["catalogEntryId", "catalogVersionHash", "catalogSchemaHash", "upstreamToolName", "riskLevel"]))
|
|
return "The tool definition changed. A new review is required.";
|
|
if (changed(["connectionId", "connectionIdentityHash", "connectionConfigHash", "connectionTransportConfigHash",
|
|
"connectionAuthorizationConfigHash", "connectionAuthorizationTransportConfigHash"]))
|
|
return "The app connection or destination changed. A new review is required.";
|
|
}
|
|
return "The reviewed action is no longer current. A new review is required.";
|
|
}
|