diff --git a/cdk/src/constructs/linear-integration.ts b/cdk/src/constructs/linear-integration.ts index f535365c0..92ccca407 100644 --- a/cdk/src/constructs/linear-integration.ts +++ b/cdk/src/constructs/linear-integration.ts @@ -55,6 +55,13 @@ const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 120; /** Webhook-processor Lambda memory (MB). */ const WEBHOOK_PROCESSOR_MEMORY_MB = 512; +/** Remove-workspace Lambda timeout (seconds). 10s matches the sibling + * link/webhook request handlers — the teardown is a bounded sequence + * (registry lookup → registry revoke → secret delete → optional row purge). + * The lookup scan is the only paginating phase, and it is bounded by the + * registry's documented tens-of-rows scale, so 10s is comfortable. */ +const REMOVE_WORKSPACE_TIMEOUT_SECONDS = 10; + /** * Properties for LinearIntegration construct. */ @@ -478,6 +485,42 @@ export class LinearIntegration extends Construct { }); this.userMappingTable.grantReadWriteData(linkFn); + // --- Workspace removal (Cognito-authenticated, admin-only) --- + // Backs `bgagent linear remove-workspace `: revokes/purges the + // registry row and deletes the per-workspace OAuth secret. Keeping the + // DDB + Secrets Manager grants on this Lambda's role — not on every CLI + // user — is the whole point of routing removal through the API (see + // issue #306). Project mappings are intentionally NOT touched: mapping + // rows carry no workspace id, so they cannot be attributed to a + // workspace (removal is by project id). + const removeWorkspaceFn = new lambda.NodejsFunction(this, 'RemoveWorkspaceFn', { + entry: path.join(handlersDir, 'linear-remove-workspace.ts'), + handler: 'handler', + runtime: Runtime.NODEJS_24_X, + architecture: Architecture.ARM_64, + timeout: Duration.seconds(REMOVE_WORKSPACE_TIMEOUT_SECONDS), + environment: { + LINEAR_WORKSPACE_REGISTRY_TABLE_NAME: this.workspaceRegistryTable.tableName, + }, + bundling: commonBundling, + }); + this.workspaceRegistryTable.grantReadWriteData(removeWorkspaceFn); + // Delete the per-workspace OAuth secret created by the CLI at setup time + // (`bgagent-linear-oauth-`). The concrete name isn't known at synth + // time (operators add workspaces by slug at runtime), so scope to the + // documented prefix — same wildcard the webhook Lambdas already use. + removeWorkspaceFn.addToRolePolicy(new iam.PolicyStatement({ + actions: ['secretsmanager:DeleteSecret'], + resources: [ + Stack.of(this).formatArn({ + service: 'secretsmanager', + resource: 'secret', + arnFormat: ArnFormat.COLON_RESOURCE_NAME, + resourceName: 'bgagent-linear-oauth-*', + }), + ], + })); + // ═══════════════════════════════════════════════════════════════════════════ // API Gateway Routes // ═══════════════════════════════════════════════════════════════════════════ @@ -500,6 +543,15 @@ export class LinearIntegration extends Construct { cognitoAuthOptions, ); + // DELETE /v1/linear/workspaces/{slug} — Cognito-authenticated, admin-only. + const workspacesResource = linear.addResource('workspaces'); + const workspaceBySlug = workspacesResource.addResource('{slug}'); + workspaceBySlug.addMethod( + 'DELETE', + new apigw.LambdaIntegration(removeWorkspaceFn, { allowTestInvoke: false }), + cognitoAuthOptions, + ); + // ═══════════════════════════════════════════════════════════════════════════ // cdk-nag suppressions // ═══════════════════════════════════════════════════════════════════════════ @@ -522,7 +574,7 @@ export class LinearIntegration extends Construct { }, ]); - const allFunctions = [webhookFn, webhookProcessorFn, linkFn]; + const allFunctions = [webhookFn, webhookProcessorFn, linkFn, removeWorkspaceFn]; for (const fn of allFunctions) { NagSuppressions.addResourceSuppressions(fn, [ { diff --git a/cdk/src/handlers/linear-remove-workspace.ts b/cdk/src/handlers/linear-remove-workspace.ts new file mode 100644 index 000000000..bcf2667e3 --- /dev/null +++ b/cdk/src/handlers/linear-remove-workspace.ts @@ -0,0 +1,472 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { DeleteSecretCommand, SecretsManagerClient } from '@aws-sdk/client-secrets-manager'; +import { DeleteCommand, ScanCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb'; +import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; +import { ulid } from 'ulid'; +import { extractUserId } from './shared/gateway'; +import type { LinearRevocationReason } from './shared/linear-oauth-resolver'; +import { logger } from './shared/logger'; +import { ErrorCode, errorResponse, successResponse } from './shared/response'; +import { makeClient, makeDocClient } from './shared/ua'; + +// Built through the attributed factory, not `new XxxClient({})` — a naked +// constructor silently drops the solution user agent (#319). +const ddb = makeDocClient(); +const sm = makeClient(SecretsManagerClient); + +// Left `string | undefined` — matching every other reader of this same var +// (`linear-webhook.ts:41`, `linear-webhook-processor.ts:88`, +// `orchestration-reconciler.ts:96`, `github-webhook-processor.ts:61`) — and +// guarded once inside the handler. A `!` would assert away a deploy misconfig +// and surface it as an opaque `TableName: undefined` SDK error instead. +const WORKSPACE_REGISTRY_TABLE = process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME; + +/** Same slug shape the CLI enforces (`SLUG_RE` in cli/src/commands/linear.ts). */ +const SLUG_RE = /^[a-zA-Z0-9_-]{4,50}$/; + +/** + * `revoked_reason` written for a deliberate operator removal. + * + * Declared here because this handler is the only writer, but typed by + * `LinearRevocationReason` so the resolver stays the single authority on the + * vocabulary — a rename there breaks this build. The import is `import type` + * on purpose: a value import would drag the entire OAuth resolver (SNS + * alerting, its DDB/Secrets Manager clients, the token-refresh path) into this + * Lambda's bundle, and would land this handler in the `agent.test.ts` + * minting-handler census, which exists to make exactly that import a test + * failure. + * + * Deliberately NOT `vault_consent_required`: that is the one revoked reason the + * resolver re-probes instead of refusing, so using it here would let a later + * successful vault probe un-latch a workspace an operator removed on purpose. + */ +const ADMIN_REMOVED_REVOCATION_REASON: LinearRevocationReason = 'admin_removed'; + +/** + * Prefix of the per-workspace OAuth secret name, so a row that never recorded + * its `oauth_secret_arn` can still be torn down: the name is deterministic. + * + * Duplicated as a literal rather than imported, and that is deliberate — the + * three other definitions live in places a bundled Lambda handler must not + * import from: `cli/src/linear-oauth.ts` (`LINEAR_OAUTH_SECRET_PREFIX`, a + * different package), `cdk/src/constructs/linear-identity-vault.ts:51` + * (`LINEAR_CREDENTIAL_PROVIDER_PREFIX`, would drag `aws-cdk-lib` into the + * function bundle), and the IAM resource pattern + * `bgagent-linear-oauth-*` at `linear-integration.ts:519`, which is what + * makes the by-name delete permitted. Changing the convention means changing + * all four. + */ +const OAUTH_SECRET_NAME_PREFIX = 'bgagent-linear-oauth-'; + +/** + * Upper bound on registry scan pages. The registry holds one row per onboarded + * workspace (tens at most), so ~20 pages is orders of magnitude of headroom; + * past it something is structurally wrong (the table outgrew the scan design) + * and a clean 500 naming the cap beats burning the 10s Lambda timeout. + */ +const MAX_SCAN_PAGES = 20; + +/** + * The response body. Declared here and applied with `satisfies` at the return + * so the shape is pinned to a type rather than to whatever the object literal + * happens to say — `LinearRemoveWorkspaceResponse` in `cli/src/types.ts` is on + * `check-types-sync.ts`'s `CLI_ONLY_ALLOWLIST` (like `LinearLinkResponse` and + * its siblings), so nothing else cross-checks the two. Keep them in step. + */ +interface RemoveWorkspaceResponseBody { + readonly workspace_slug: string; + readonly linear_workspace_id: string; + readonly status: 'revoked' | 'purged'; + readonly secret: 'deleted' | 'absent' | 'not_applicable'; + readonly provider_name?: string; +} + +/** + * DELETE /v1/linear/workspaces/{slug} — deregister a Linear workspace. + * + * Cognito-authenticated. Only the workspace admin (the platform user who + * ran `bgagent linear setup`/`add-workspace` for the slug, recorded as + * `installed_by_platform_user_id`) may remove it. + * + * By default this is a *soft* removal that preserves the audit trail: + * 1. Flip the registry row to `status='revoked'` with + * `revoked_reason='admin_removed'`. The OAuth resolver refuses any + * non-active row (`shared/linear-oauth-resolver.ts`) with one documented + * exception — a `revoked` row whose reason is `vault_consent_required` is + * re-probed rather than refused (`:392-394`). `admin_removed` is + * deliberately not that reason, so an admin removal is terminal: the + * workspace stops resolving tokens and routing webhooks the instant this + * write lands, and no later vault probe can un-latch it. + * 2. Delete the per-workspace `bgagent-linear-oauth-` secret so no + * credential lingers. + * + * Query flag: + * - `purge=true` — delete the registry row outright (no audit row). + * + * Idempotent on the secret, and the response says *which* of three things + * happened rather than collapsing them into one boolean: + * - `secret: 'deleted'` — a live secret was destroyed here. + * - `secret: 'absent'` — nothing to delete (a prior partial run, or + * a row that never recorded an ARN and has no + * secret under the deterministic name). + * - `secret: 'not_applicable'` — this workspace is vault-managed and never + * had a Secrets Manager secret of its own. + * + * **Vault-managed workspaces are NOT fully torn down by this endpoint.** A row + * with `provider_name` was onboarded through AgentCore Identity, and its + * **credential provider lives outside CloudFormation**, holding the Linear + * client secret and a live, self-refreshing grant. This handler does not touch + * it — a cross-service teardown with its own failure modes, tracked separately. + * It *reports* it: `provider_name` is echoed in the response so the CLI can + * print the exact `delete-oauth2-credential-provider` follow-up. Without that, + * the vault path renders byte-identically to a clean teardown while a + * self-refreshing credential survives. + * + * Project mappings are NOT touched here: `LinearProjectMappingTable` rows + * carry no workspace identifier (the `onboard-project` writer records only + * `linear_project_id`), so they cannot be attributed to a workspace. Removing + * a mapping is a by-project-id operation (see LINEAR_SETUP_GUIDE). A follow-up + * will record `linear_workspace_id` at onboard time to enable workspace-scoped + * cleanup. + */ +export async function handler(event: APIGatewayProxyEvent): Promise { + const requestId = ulid(); + // Outer-scope breadcrumbs so the top-level catch can name the workspace + // and the phase that failed — the difference between "which secret + // leaked?" being answerable from one log line vs. a manual hunt. + let slug = ''; + let phase: 'lookup' | 'registry_write' | 'secret_delete' = 'lookup'; + + try { + const userId = extractUserId(event); + if (!userId) { + return errorResponse(401, ErrorCode.UNAUTHORIZED, 'Authentication required.', requestId); + } + + slug = (event.pathParameters?.slug ?? '').trim(); + if (!SLUG_RE.test(slug)) { + return errorResponse( + 400, + ErrorCode.VALIDATION_ERROR, + 'Invalid workspace slug. Must be 4-50 chars matching [a-zA-Z0-9_-].', + requestId, + ); + } + + const purge = event.queryStringParameters?.purge === 'true'; + + if (!WORKSPACE_REGISTRY_TABLE) { + // Deploy misconfiguration, not a caller error. Log it as itself instead + // of letting the SDK reject `TableName: undefined` from inside the scan. + logger.error('LINEAR_WORKSPACE_REGISTRY_TABLE_NAME is not set — cannot service a workspace removal', { + request_id: requestId, + workspace_slug: slug, + }); + return errorResponse(500, ErrorCode.INTERNAL_ERROR, 'Internal server error.', requestId); + } + const registryTable = WORKSPACE_REGISTRY_TABLE; + + // ─── Locate the registry row by slug ───────────────────────────── + // The registry table is keyed on `linear_workspace_id`, so a slug + // lookup is a filtered scan. Only `status='active'` rows are valid + // removal targets — an already-revoked (or unknown) slug returns 404, + // so the endpoint does not distinguish revoked from missing and the + // destructive path is not re-run on a row that's already torn down. + // The scan is `ConsistentRead` and the revoke below carries a + // `ConditionExpression`, because a filtered scan alone is a TOCTOU: + // read-your-writes matters when a removal lands seconds after + // `add-workspace`, and the condition is what actually settles two + // concurrent DELETEs. + // + // No `Limit`: DynamoDB applies a FilterExpression *after* evaluating + // items, so `Limit: N` bounds items examined, not items matched — a + // filtered `Limit: 1` scan can return `[]` + a LastEvaluatedKey while + // the target sits one page deeper (the normal shared-stack state once + // the registry holds more than one row). We follow the continuation key + // until a match or key exhaustion (not to completion — the loop stops on + // the first match), matching the paginated small-table scan of this same + // registry shape in `shared/jira-tenant-registry.ts:32-46`, which is + // also where the `ConsistentRead` precedent comes from. The registry + // holds one row per onboarded workspace and stays small (tens of rows at + // most); if it ever grows large, add a GSI on `workspace_slug` and Query + // it. (`shared/linear-issue-lookup.ts:131-136` scans this table + // *without* pagination — a counter-example, not the precedent, and worth + // its own fix.) + let row: Record | undefined; + let scanKey: Record | undefined; + let pages = 0; + do { + const page = await ddb.send(new ScanCommand({ + TableName: registryTable, + FilterExpression: 'workspace_slug = :slug AND #status = :active', + ExpressionAttributeNames: { '#status': 'status' }, + ExpressionAttributeValues: { ':slug': slug, ':active': 'active' }, + ExclusiveStartKey: scanKey, + ConsistentRead: true, + })); + row = page.Items?.[0]; + scanKey = page.LastEvaluatedKey as Record | undefined; + pages += 1; + if (!row && scanKey && pages >= MAX_SCAN_PAGES) { + // Bounded rather than open-ended: without this the only stop is the + // 10s timeout, which surfaces as a generic 500 with no cause. Not a + // 404 — the row may well exist further in, and claiming it doesn't + // would be the same lie this endpoint is being fixed to stop telling. + logger.error('Linear registry scan hit the page cap without finding the workspace', { + request_id: requestId, + workspace_slug: slug, + pages, + max_pages: MAX_SCAN_PAGES, + }); + return errorResponse(500, ErrorCode.INTERNAL_ERROR, 'Internal server error.', requestId); + } + } while (!row && scanKey); + if (!row) { + // Collapse "no such row" and "already revoked" into one 404 — the + // caller learns nothing about existence, and there's nothing left + // to remove either way. + return errorResponse(404, ErrorCode.WORKSPACE_NOT_FOUND, `Workspace '${slug}' is not an active registration.`, requestId); + } + + // ─── Admin authorization ───────────────────────────────────────── + const installedBy = row.installed_by_platform_user_id as string | undefined; + if (installedBy !== userId) { + logger.warn('Linear remove-workspace rejected: caller is not the workspace admin', { + request_id: requestId, + workspace_slug: slug, + }); + return errorResponse(403, ErrorCode.FORBIDDEN, 'Only the workspace admin who installed this workspace may remove it.', requestId); + } + + const linearWorkspaceId = row.linear_workspace_id as string; + const oauthSecretArn = row.oauth_secret_arn as string | undefined; + // Present only on rows onboarded through the AgentCore Identity vault. Read + // (and reported) but never deleted here — see the header note. + const providerName = row.provider_name as string | undefined; + const now = new Date().toISOString(); + + // Track which teardown phase we're in so a mid-stream failure logs + // *where* it broke — critical because the registry row is revoked + // first (fail-closed), so a later failure can leave a live OAuth + // secret orphaned. On-call needs the phase + workspace id from the + // error log to find and hand-purge it. + phase = 'registry_write'; + + // ─── Registry: revoke first (fail-closed), always ──────────────── + // Even on `--purge` we flip the row to `status='revoked'` BEFORE + // deleting the secret, rather than deleting the row outright. This is + // deliberate: the OAuth resolver refuses a non-active row — and + // `revoked_reason='admin_removed'` is specifically not the one reason it + // re-probes instead of refusing (`linear-oauth-resolver.ts:392-394`) — so + // the workspace stops resolving tokens and routing webhooks the instant + // this write lands, terminally. It also keeps the row present through the + // secret-delete step, so a failure there can persist a durable + // orphaned-secret marker on the row (see `markSecretDeletionFailed`). + // The hard `--purge` delete of the row happens only AFTER the secret + // is confirmed gone. + // + // The `ConditionExpression` is what closes the scan's TOCTOU: it makes + // "the row was active when we decided to remove it" a property of the + // write, not of a read that happened earlier. Two concurrent DELETEs now + // resolve to one revoke and one 404 instead of both proceeding. + try { + await ddb.send(new UpdateCommand({ + TableName: registryTable, + Key: { linear_workspace_id: linearWorkspaceId }, + UpdateExpression: 'SET #status = :revoked, revoked_reason = :reason, revoked_at = :now, revoked_by_platform_user_id = :uid, updated_at = :now', + ConditionExpression: '#status = :active', + ExpressionAttributeNames: { '#status': 'status' }, + ExpressionAttributeValues: { + ':revoked': 'revoked', + ':reason': ADMIN_REMOVED_REVOCATION_REASON, + ':active': 'active', + ':now': now, + ':uid': userId, + }, + })); + } catch (err) { + if ((err as { name?: string }).name !== 'ConditionalCheckFailedException') throw err; + // Someone else moved the row out of `active` between our scan and this + // write — a concurrent DELETE, or the resolver latching the row revoked + // after Linear rejected its refresh token. Either way this request + // changed nothing, so 404 (the same answer a caller gets for an + // already-revoked slug) is the truthful one. Log the secret ARN at WARN: + // if the winner was the resolver rather than a peer DELETE, nobody + // deleted the secret, and this line is what makes that orphan findable. + logger.warn('Linear remove-workspace lost a race: the registry row was no longer active at write time', { + request_id: requestId, + workspace_slug: slug, + linear_workspace_id: linearWorkspaceId, + oauth_secret_arn: oauthSecretArn, + }); + return errorResponse(404, ErrorCode.WORKSPACE_NOT_FOUND, `Workspace '${slug}' is not an active registration.`, requestId); + } + + // ─── Secrets Manager: delete the per-workspace OAuth secret ─────── + // Idempotent: a ResourceNotFoundException means the secret was already + // removed by a prior (partial) run — that's success, not an error. + // + // The delete is attempted even when the row recorded no `oauth_secret_arn`, + // because the secret name is deterministic (`bgagent-linear-oauth-`) + // and the function's IAM grant is a prefix grant over exactly that shape. + // A `setup` that created the secret and then died before finishing the row + // leaves precisely this state, and the old code skipped it — reporting + // "already absent" about a secret it never looked for. + phase = 'secret_delete'; + const secretId = oauthSecretArn ?? `${OAUTH_SECRET_NAME_PREFIX}${slug}`; + let secret: RemoveWorkspaceResponseBody['secret']; + try { + await sm.send(new DeleteSecretCommand({ + SecretId: secretId, + // No recovery window — the workspace is being torn down and the + // registry row is the audit record. Leaving a scheduled-deletion + // secret around would block a same-slug re-onboarding. + ForceDeleteWithoutRecovery: true, + })); + secret = 'deleted'; + } catch (err) { + const name = (err as { name?: string }).name; + if (name !== 'ResourceNotFoundException') { + // A real SM failure (e.g. AccessDenied, throttle). The registry + // row is already revoked (fail-closed holds) AND still present + // (the `--purge` delete has not run yet), so we persist a durable + // marker on the row (best-effort) so the leaked secret is + // discoverable and the operator can hand-purge it, then surface a + // distinct, actionable error instead of an opaque 500. We do NOT + // proceed to the `--purge` row delete — deleting the row here + // would strip the only durable record of the orphaned secret. Do + // NOT swallow. + await markSecretDeletionFailed(registryTable, linearWorkspaceId, secretId, name) + .catch((markErr) => logger.error('Failed to persist secret-deletion-failed marker', { + request_id: requestId, + linear_workspace_id: linearWorkspaceId, + error: markErr instanceof Error ? markErr.message : String(markErr), + })); + logger.error('Linear OAuth secret delete failed — workspace revoked but secret must be manually purged', { + request_id: requestId, + workspace_slug: slug, + linear_workspace_id: linearWorkspaceId, + oauth_secret_id: secretId, + error_name: name, + }); + return errorResponse( + 500, + ErrorCode.SECRET_DELETE_FAILED, + `Workspace '${slug}' was revoked but its OAuth secret could not be deleted. ` + + 'The workspace is disabled (fail-closed), but an operator must manually delete ' + + `the Secrets Manager secret. Request ID ${requestId}.`, + requestId, + ); + } + // Nothing under that name. Two different facts share this branch, and + // the response distinguishes them: + // - the row recorded an ARN, or the workspace isn't vault-managed → + // there was supposed to be a secret here and it's gone: `absent`. + // - vault-managed AND no ARN was ever recorded → the credential lives + // in the AgentCore provider, not in Secrets Manager, so there was + // never a per-workspace secret to delete: `not_applicable`. + // Collapsing these was the bug: `not_applicable` is the one case where + // "nothing was deleted" does NOT mean teardown is complete. + secret = providerName && !oauthSecretArn ? 'not_applicable' : 'absent'; + logger.info('Linear OAuth secret not present at removal time', { + request_id: requestId, + workspace_slug: slug, + oauth_secret_id: secretId, + secret, + }); + } + + // ─── Registry: purge (hard delete) ─────────────────────────────── + // Only on `--purge`, and only now that the secret is confirmed gone — + // so we never delete the audit/marker row while a live secret could + // still be orphaned. + if (purge) { + phase = 'registry_write'; + await ddb.send(new DeleteCommand({ + TableName: registryTable, + Key: { linear_workspace_id: linearWorkspaceId }, + })); + } + + logger.info('Linear workspace removed', { + request_id: requestId, + workspace_slug: slug, + linear_workspace_id: linearWorkspaceId, + mode: purge ? 'purged' : 'revoked', + secret, + // Present ⇒ an AgentCore credential provider survives this removal and + // an operator still has to delete it. Logged so the follow-up is + // reconstructable from CloudWatch alone, not only from the CLI output + // the operator may have scrolled past. + ...(providerName && { vault_provider_name: providerName }), + }); + + return successResponse(200, { + workspace_slug: slug, + linear_workspace_id: linearWorkspaceId, + status: purge ? 'purged' : 'revoked', + secret, + ...(providerName && { provider_name: providerName }), + } satisfies RemoveWorkspaceResponseBody, requestId); + } catch (err) { + // Include the workspace slug + failing phase so on-call can locate an + // orphaned secret / half-cleaned mapping table from the error log. + logger.error('Linear remove-workspace handler failed', { + error: err instanceof Error ? err.message : String(err), + request_id: requestId, + workspace_slug: slug, + phase, + }); + return errorResponse(500, ErrorCode.INTERNAL_ERROR, 'Internal server error.', requestId); + } +} + +/** + * Best-effort durable marker written to the registry row when the OAuth + * secret delete fails after the row was already revoked. The registry row is + * always still present at this point — the revoke is an `UpdateCommand` and + * the `--purge` row delete runs only after the secret is confirmed gone — so + * the marker survives on every flag combination and makes the orphaned-secret + * condition discoverable. Never throws to the caller — the caller already + * logs + returns an actionable error. + * + * `oauthSecretId` is whatever was handed to `DeleteSecret` — the recorded ARN + * when the row had one, otherwise the deterministic name. Recording the name in + * that second case is the point: it is the identifier an operator can act on. + */ +async function markSecretDeletionFailed( + registryTable: string, + linearWorkspaceId: string, + oauthSecretId: string, + errorName: string | undefined, +): Promise { + await ddb.send(new UpdateCommand({ + TableName: registryTable, + Key: { linear_workspace_id: linearWorkspaceId }, + UpdateExpression: + 'SET secret_deletion_failed = :t, secret_deletion_error = :e, orphaned_oauth_secret_arn = :arn', + ExpressionAttributeValues: { + ':t': true, + ':e': errorName ?? 'unknown', + ':arn': oauthSecretId, + }, + })); +} diff --git a/cdk/src/handlers/shared/linear-oauth-resolver.ts b/cdk/src/handlers/shared/linear-oauth-resolver.ts index b44b0e6cc..a3ea9281d 100644 --- a/cdk/src/handlers/shared/linear-oauth-resolver.ts +++ b/cdk/src/handlers/shared/linear-oauth-resolver.ts @@ -64,8 +64,32 @@ const SECRET_CACHE_TTL_MS = 60_000; /** Refresh threshold: refresh tokens with <60s remaining. */ const REFRESH_THRESHOLD_SECONDS = 60; -/** Why a registry row was latched `revoked`. */ -type LinearRevocationReason = 'refresh_token_rejected' | 'vault_consent_required'; +/** + * Why a registry row was latched `revoked`. + * + * The complete vocabulary, so the re-probe guard below can be read as an + * exhaustive statement about *every* reason a row can carry. Two members are + * written by this module (see the constants under it); `admin_removed` is + * written by `linear-remove-workspace.ts` when an operator deliberately + * deregisters a workspace, and is deliberately NOT + * `VAULT_CONSENT_REVOCATION_REASON` — that is the one reason the guard + * re-probes instead of refusing, so an admin removal stays terminal: no later + * vault probe can un-latch it. "We inferred the grant is gone" and "a human + * said take this workspace out" must not be the same string. + * + * Exported as a **type only** on purpose. The removal handler needs the + * vocabulary but must not take a value dependency on this module: a value + * import would pull the whole resolver (SNS alerting, DDB + Secrets Manager + * clients, the refresh path) into that Lambda's esbuild bundle, and it would + * register in the `agent.test.ts` "no Linear-minting handler is unwired" + * census, whose value is precisely that a new value import there is a test + * failure rather than a production 401. `import type` is erased, so it costs + * nothing at runtime and still fails the build if this union changes. + */ +export type LinearRevocationReason = + | 'refresh_token_rejected' + | 'vault_consent_required' + | 'admin_removed'; /** * `revoked_reason` written when the vault answered with an authorization URL @@ -80,6 +104,11 @@ const VAULT_CONSENT_REVOCATION_REASON: LinearRevocationReason = 'vault_consent_r /** `revoked_reason` written when Linear itself rejected the refresh token. */ const REFRESH_REJECTED_REVOCATION_REASON: LinearRevocationReason = 'refresh_token_rejected'; +// No constant for `admin_removed`: this module never writes it. Its writer +// declares it locally, typed by the union above, in +// `handlers/linear-remove-workspace.ts` — see the note on the union for why +// that direction of the dependency is type-only. + /** Registry row status values. Anything else (missing, unknown * string) is treated as `revoked` so a corrupt or partially-written * row blocks resolution rather than silently granting access. */ diff --git a/cdk/src/handlers/shared/response.ts b/cdk/src/handlers/shared/response.ts index 4c92d1f03..bab4d5b4c 100644 --- a/cdk/src/handlers/shared/response.ts +++ b/cdk/src/handlers/shared/response.ts @@ -35,6 +35,8 @@ export const ErrorCode = { WEBHOOK_ALREADY_REVOKED: 'WEBHOOK_ALREADY_REVOKED', API_KEY_NOT_FOUND: 'API_KEY_NOT_FOUND', API_KEY_ALREADY_REVOKED: 'API_KEY_ALREADY_REVOKED', + WORKSPACE_NOT_FOUND: 'WORKSPACE_NOT_FOUND', + SECRET_DELETE_FAILED: 'SECRET_DELETE_FAILED', REPO_NOT_ONBOARDED: 'REPO_NOT_ONBOARDED', BUDGET_EXCEEDED: 'BUDGET_EXCEEDED', SERVICE_UNAVAILABLE: 'SERVICE_UNAVAILABLE', diff --git a/cdk/test/constructs/linear-integration.test.ts b/cdk/test/constructs/linear-integration.test.ts index 55f20f791..9638d24e1 100644 --- a/cdk/test/constructs/linear-integration.test.ts +++ b/cdk/test/constructs/linear-integration.test.ts @@ -64,14 +64,114 @@ describe('LinearIntegration construct', () => { }); }); - test('creates three Lambda functions (webhook, processor, link)', () => { - template.resourceCountIs('AWS::Lambda::Function', 3); + test('creates four Lambda functions (webhook, processor, link, remove-workspace)', () => { + template.resourceCountIs('AWS::Lambda::Function', 4); }); test('creates API Gateway resources under /linear', () => { template.hasResourceProperties('AWS::ApiGateway::Resource', { PathPart: 'linear' }); template.hasResourceProperties('AWS::ApiGateway::Resource', { PathPart: 'webhook' }); template.hasResourceProperties('AWS::ApiGateway::Resource', { PathPart: 'link' }); + template.hasResourceProperties('AWS::ApiGateway::Resource', { PathPart: 'workspaces' }); + template.hasResourceProperties('AWS::ApiGateway::Resource', { PathPart: '{slug}' }); + }); + + test('DELETE /linear/workspaces/{slug} is Cognito-authorized (pinned to the {slug} resource)', () => { + // Pin the method to the {slug} resource so "any authorized DELETE + // anywhere" cannot satisfy this — the DELETE must be on the + // workspace-by-slug path specifically. + const slugResources = template.findResources('AWS::ApiGateway::Resource', { + Properties: { PathPart: '{slug}' }, + }); + const slugLogicalIds = Object.keys(slugResources); + expect(slugLogicalIds).toHaveLength(1); + const slugId = slugLogicalIds[0]; + + const deleteMethods = template.findResources('AWS::ApiGateway::Method', { + Properties: { HttpMethod: 'DELETE' }, + }); + const onSlug = Object.values(deleteMethods).filter( + (m) => (m.Properties as { ResourceId?: { Ref?: string } }).ResourceId?.Ref === slugId, + ); + expect(onSlug).toHaveLength(1); + expect((onSlug[0].Properties as { AuthorizationType?: string }).AuthorizationType).toBe('COGNITO_USER_POOLS'); + }); + + // Locate the RemoveWorkspaceFn INDEPENDENTLY of any IAM policy: it is the + // ONLY Lambda whose environment is registry-only — it carries + // `LINEAR_WORKSPACE_REGISTRY_TABLE_NAME` but none of the sibling markers + // (`LINEAR_PROJECT_MAPPING_TABLE_NAME` on the processor, + // `LINEAR_WEBHOOK_SECRET_ARN` on the webhook receiver, + // `LINEAR_USER_MAPPING_TABLE_NAME` on the link handler). We then read the + // role off the FUNCTION resource itself (`Role: Fn::GetAtt[]`), so + // the derived `role` is bound to the function's own identity — NOT read out + // of the DeleteSecret policy. This lets the secret-prefix test assert the + // grant lands on THIS role and genuinely fail if a future edit attaches the + // DeleteSecret grant to the wrong role. + function findRemoveWorkspaceFn(): { logicalId: string; role: string } { + const fns = template.findResources('AWS::Lambda::Function'); + const matches = Object.entries(fns).filter(([, fn]) => { + const vars = + (fn.Properties as { Environment?: { Variables?: Record } }) + .Environment?.Variables ?? {}; + return ( + 'LINEAR_WORKSPACE_REGISTRY_TABLE_NAME' in vars && + !('LINEAR_PROJECT_MAPPING_TABLE_NAME' in vars) && + !('LINEAR_WEBHOOK_SECRET_ARN' in vars) && + !('LINEAR_USER_MAPPING_TABLE_NAME' in vars) + ); + }); + expect(matches).toHaveLength(1); + const [logicalId, fn] = matches[0]; + const role = (fn.Properties as { Role?: { 'Fn::GetAtt'?: [string, string] } }) + .Role?.['Fn::GetAtt']?.[0]; + expect(role).toBeDefined(); + return { logicalId, role: role! }; + } + + test('remove-workspace handler wires ONLY the workspace registry (no project mapping table)', () => { + // B2: the mapping-cleanup path was dropped, so the remove-workspace + // function must NOT carry the project-mapping table env var (that was + // the dead grant + no-op cleanup the reviewer flagged). + const { logicalId } = findRemoveWorkspaceFn(); + const fn = template.findResources('AWS::Lambda::Function')[logicalId]; + const vars = (fn.Properties as { Environment: { Variables: Record } }) + .Environment.Variables; + expect(vars).toHaveProperty('LINEAR_WORKSPACE_REGISTRY_TABLE_NAME'); + expect(vars).not.toHaveProperty('LINEAR_PROJECT_MAPPING_TABLE_NAME'); + }); + + test('remove-workspace role can delete ONLY the bgagent-linear-oauth-* secret prefix (scope pinned to the role)', () => { + // Bind the DeleteSecret grant to the remove-workspace role AND pin the + // resource ARN to the bgagent-linear-oauth-* prefix, so a future + // widening of that wildcard (or attaching DeleteSecret to another role) + // fails this test. + const { role } = findRemoveWorkspaceFn(); + const policies = template.findResources('AWS::IAM::Policy'); + const deletePolicies = Object.values(policies).filter((p) => { + const doc = (p.Properties as { PolicyDocument: { Statement: Array<{ Action?: unknown }> } }) + .PolicyDocument; + return doc.Statement.some((s) => { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + return actions.includes('secretsmanager:DeleteSecret'); + }); + }); + expect(deletePolicies).toHaveLength(1); + + const policy = deletePolicies[0]; + // The policy is attached to the remove-workspace role only. + const roleRefs = ((policy.Properties as { Roles?: Array<{ Ref?: string }> }).Roles ?? []) + .map((r) => r.Ref); + expect(roleRefs).toContain(role); + + // The DeleteSecret statement's resource ends with the documented prefix. + const stmt = (policy.Properties as { + PolicyDocument: { Statement: Array<{ Action?: unknown; Resource?: unknown }> }; + }).PolicyDocument.Statement.find((s) => { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + return actions.includes('secretsmanager:DeleteSecret'); + })!; + expect(JSON.stringify(stmt.Resource)).toContain('bgagent-linear-oauth-*'); }); test('creates one Secrets Manager secret (webhook signing) — OAuth tokens are CLI-created at runtime', () => { diff --git a/cdk/test/handlers/linear-remove-workspace.test.ts b/cdk/test/handlers/linear-remove-workspace.test.ts new file mode 100644 index 000000000..ac5174c87 --- /dev/null +++ b/cdk/test/handlers/linear-remove-workspace.test.ts @@ -0,0 +1,650 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import type { APIGatewayProxyEvent } from 'aws-lambda'; + +const ddbSend = jest.fn(); +const smSend = jest.fn(); + +jest.mock('@aws-sdk/client-dynamodb', () => ({ DynamoDBClient: jest.fn(() => ({})) })); +jest.mock('@aws-sdk/lib-dynamodb', () => ({ + DynamoDBDocumentClient: { from: jest.fn(() => ({ send: ddbSend })) }, + ScanCommand: jest.fn((input: Record) => ({ _type: 'Scan', input })), + UpdateCommand: jest.fn((input: Record) => ({ _type: 'Update', input })), + DeleteCommand: jest.fn((input: Record) => ({ _type: 'Delete', input })), +})); +jest.mock('@aws-sdk/client-secrets-manager', () => ({ + SecretsManagerClient: jest.fn(() => ({ send: smSend })), + DeleteSecretCommand: jest.fn((input: unknown) => ({ _type: 'DeleteSecret', input })), +})); + +jest.mock('ulid', () => ({ ulid: jest.fn(() => 'REQ-ULID') })); + +process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = 'LinearRegistry'; + +import { handler } from '../../src/handlers/linear-remove-workspace'; + +const ADMIN = 'cognito-admin-sub'; + +function makeEvent(opts: { + slug?: string; + userId?: string; + query?: Record; +} = {}): APIGatewayProxyEvent { + return { + body: null, + headers: {}, + multiValueHeaders: {}, + httpMethod: 'DELETE', + isBase64Encoded: false, + path: `/v1/linear/workspaces/${opts.slug ?? 'acme'}`, + pathParameters: opts.slug === undefined ? { slug: 'acme' } : { slug: opts.slug }, + queryStringParameters: opts.query ?? null, + multiValueQueryStringParameters: null, + stageVariables: null, + requestContext: opts.userId + ? ({ authorizer: { claims: { sub: opts.userId } } } as unknown as APIGatewayProxyEvent['requestContext']) + : ({} as APIGatewayProxyEvent['requestContext']), + resource: '', + }; +} + +function activeRow(overrides: Record = {}) { + return { + linear_workspace_id: 'ws-uuid-1', + workspace_slug: 'acme', + oauth_secret_arn: 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-linear-oauth-acme-AbCd', + installed_by_platform_user_id: ADMIN, + status: 'active', + ...overrides, + }; +} + +/** + * Route DDB commands by type + table rather than by call order, so a test + * only has to declare the registry contents it cares about. + * + * The registry scan double models *real* DynamoDB filtered-Scan semantics — + * this is what makes B1 (`Limit: 1` on a filtered scan) observable and the + * two-active-workspaces regression expressible: + * 1. `Limit` bounds the items *examined* (the raw page slice), NOT the + * items matched — DynamoDB applies the FilterExpression AFTER slicing. + * 2. `ExclusiveStartKey` advances a page cursor over the seeded rows. + * 3. `LastEvaluatedKey` is returned whenever unexamined rows remain, even + * if this page matched nothing. + * The seeded rows are held in table order; the handler's filter (slug + + * `status='active'`) is applied to the examined slice. A handler that + * examines only one arbitrary row (Limit: 1) and never follows the key can + * therefore miss a matching row that sits on a later page. + */ +function routeDdb(opts: { + registryRow?: Record | null; + registryRows?: Record[]; +} = {}) { + const rows: Record[] = opts.registryRows + ?? (opts.registryRow === undefined + ? [activeRow()] + : (opts.registryRow === null ? [] : [opts.registryRow])); + + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Scan' && cmd.input.TableName === 'LinearRegistry') { + const slug = (cmd.input.ExpressionAttributeValues as Record)?.[':slug']; + const start = Number((cmd.input.ExclusiveStartKey as { _idx?: number } | undefined)?._idx ?? 0); + const limit = cmd.input.Limit as number | undefined; + const end = limit === undefined ? rows.length : Math.min(rows.length, start + limit); + const examined = rows.slice(start, end); + // Apply the handler's FilterExpression to the examined slice only. + const matched = examined.filter((r) => r.status === 'active' && r.workspace_slug === slug); + const more = end < rows.length; + return Promise.resolve({ + Items: matched, + ...(more ? { LastEvaluatedKey: { _idx: end } } : {}), + }); + } + return Promise.resolve({}); + }); +} + +/** + * Find the revoke Update by the value bound to `:revoked`, not by a + * `JSON.stringify(...).includes('revoked')` substring — the literal "revoked" + * appears in three *attribute names* the same command writes (`revoked_at`, + * `revoked_reason`, `revoked_by_platform_user_id`), so a stringify match would + * still pass if the status were never set at all. + */ +function findRevoke() { + return ddbSend.mock.calls.find( + ([c]) => c._type === 'Update' + && c.input.TableName === 'LinearRegistry' + && (c.input.ExpressionAttributeValues as Record | undefined)?.[':revoked'] === 'revoked', + ); +} + +/** Find the orphaned-secret marker Update by its UpdateExpression target. */ +function findMarker() { + return ddbSend.mock.calls.find( + ([c]) => c._type === 'Update' + && String(c.input.UpdateExpression ?? '').includes('secret_deletion_failed'), + ); +} + +describe('linear-remove-workspace handler', () => { + beforeEach(() => { + ddbSend.mockReset(); + smSend.mockReset(); + smSend.mockResolvedValue({}); + }); + + test('401s without a Cognito JWT', async () => { + const result = await handler(makeEvent({ slug: 'acme' })); + expect(result.statusCode).toBe(401); + }); + + test('400s on an invalid slug', async () => { + const result = await handler(makeEvent({ slug: 'a', userId: ADMIN })); + expect(result.statusCode).toBe(400); + }); + + test('404s when the workspace is not in the registry', async () => { + routeDdb({ registryRow: null }); + const result = await handler(makeEvent({ slug: 'ghost', userId: ADMIN })); + expect(result.statusCode).toBe(404); + }); + + test('403s when the caller is not the workspace admin', async () => { + routeDdb(); + const result = await handler(makeEvent({ slug: 'acme', userId: 'not-the-admin' })); + expect(result.statusCode).toBe(403); + // Must NOT have deleted the secret or written the row. + expect(smSend).not.toHaveBeenCalled(); + expect(ddbSend.mock.calls.filter(([c]) => c._type === 'Update' || c._type === 'Delete')).toHaveLength(0); + }); + + test('happy path: revokes the registry row and deletes the secret (default flags)', async () => { + routeDdb(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + + // Registry row flipped to revoked, NOT deleted, by default. Asserted on + // the individual attribute values rather than a JSON.stringify substring: + // 'revoked' also appears in the attribute *names* (`revoked_at`, + // `revoked_reason`, `revoked_by_platform_user_id`), so a stringify match + // passes even if `:revoked` were never bound to the status. + const updateCall = ddbSend.mock.calls.find(([c]) => c._type === 'Update'); + expect(updateCall).toBeTruthy(); + expect(updateCall![0].input.Key).toEqual({ linear_workspace_id: 'ws-uuid-1' }); + expect(updateCall![0].input.ExpressionAttributeValues).toMatchObject({ + ':revoked': 'revoked', + // Terminal, and deliberately NOT `vault_consent_required` — that is the + // one revoked reason the OAuth resolver re-probes instead of refusing. + ':reason': 'admin_removed', + ':uid': ADMIN, + }); + // The revoke is conditional on the row still being active, so two + // concurrent DELETEs can't both believe they won (a filtered Scan is a + // TOCTOU on its own; only the condition settles it). + expect(updateCall![0].input.ConditionExpression).toBe('#status = :active'); + expect(ddbSend.mock.calls.filter(([c]) => c._type === 'Delete')).toHaveLength(0); + + // Secret deleted — and asserted on *what* was deleted and *how*. A bare + // "a DeleteSecret happened" would pass on a wrong SecretId, and without + // ForceDeleteWithoutRecovery the secret lingers for a 7-30 day recovery + // window while the caller is told teardown is done. + const secretCall = smSend.mock.calls.find(([c]) => c._type === 'DeleteSecret'); + expect(secretCall).toBeTruthy(); + expect(secretCall![0].input).toEqual({ + SecretId: 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-linear-oauth-acme-AbCd', + ForceDeleteWithoutRecovery: true, + }); + + const body = JSON.parse(result.body) as { data: { status: string; secret: string } }; + expect(body.data.status).toBe('revoked'); + expect(body.data.secret).toBe('deleted'); + }); + + test('--purge deletes the registry row (after a fail-closed revoke) and reports purged', async () => { + routeDdb(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN, query: { purge: 'true' } })); + expect(result.statusCode).toBe(200); + + // The row is revoked first (fail-closed) and then hard-deleted — so both + // an Update and a Delete land on the registry row on the purge path. + const updateCall = ddbSend.mock.calls.find(([c]) => c._type === 'Update'); + expect(updateCall).toBeTruthy(); + expect(updateCall![0].input.ExpressionAttributeValues).toMatchObject({ ':revoked': 'revoked' }); + const deleteCall = ddbSend.mock.calls.find(([c]) => c._type === 'Delete'); + expect(deleteCall).toBeTruthy(); + expect(deleteCall![0].input.Key).toEqual({ linear_workspace_id: 'ws-uuid-1' }); + + const body = JSON.parse(result.body) as { data: { status: string } }; + expect(body.data.status).toBe('purged'); + }); + + test('secret-already-gone is idempotent (ResourceNotFoundException swallowed)', async () => { + routeDdb(); + smSend.mockReset(); + smSend.mockRejectedValueOnce( + Object.assign(new Error('not found'), { name: 'ResourceNotFoundException' }), + ); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + const body = JSON.parse(result.body) as { data: { secret: string; status: string; provider_name?: string } }; + // Row still revoked; the secret this row *recorded* was already gone, so + // teardown is genuinely complete → `absent`, not `not_applicable`. + expect(body.data.status).toBe('revoked'); + expect(body.data.secret).toBe('absent'); + expect(body.data).not.toHaveProperty('provider_name'); + }); + + test('never touches a project-mapping table (mapping cleanup dropped)', async () => { + // Mapping cleanup was removed: mapping rows carry no workspace id, so + // they can't be attributed to a workspace. The handler must not scan or + // delete any mapping table, and the response carries no mapping count. + routeDdb(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + + // The only table the handler touches is the registry. + const nonRegistry = ddbSend.mock.calls.filter( + ([c]) => c.input?.TableName !== 'LinearRegistry', + ); + expect(nonRegistry).toHaveLength(0); + + const body = JSON.parse(result.body) as { data: Record }; + expect(body.data).not.toHaveProperty('mappings_removed'); + }); + + test('B1 regression: finds a live workspace that is not the first registry row (two active rows, target second)', async () => { + // Two active rows on one shared stack (the normal multi-workspace state). + // The double models real filtered-Scan semantics: with a `Limit: 1` scan + // it would examine only the first row (`ws-other`), filter it out, and + // return `[]` + a LastEvaluatedKey — so the old `Limit: 1` handler that + // read only `Items[0]` and never followed the key would 404 the live + // target. The fixed handler sends no Limit, so the filter matches the + // second row and the revoke lands on it. This test 404s before the fix + // and passes after. + routeDdb({ + registryRows: [ + activeRow({ linear_workspace_id: 'ws-other', workspace_slug: 'other' }), + activeRow({ linear_workspace_id: 'ws-acme', workspace_slug: 'acme' }), + ], + }); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + + // The revoke landed on the *target* row, not the first-examined one. + const revoke = ddbSend.mock.calls.find(([c]) => c._type === 'Update'); + expect(revoke![0].input.Key).toEqual({ linear_workspace_id: 'ws-acme' }); + + const body = JSON.parse(result.body) as { data: { linear_workspace_id: string } }; + expect(body.data.linear_workspace_id).toBe('ws-acme'); + }); + + test('registry scan follows LastEvaluatedKey across pages (no Limit)', async () => { + // Directly asserts the handler paginates. We hand-roll the Scan double + // (rather than reuse routeDdb) so we can split the registry across two + // pages: page one is empty but carries a continuation key, and the target + // sits on page two. A handler that reads only `Items[0]` on the first + // page and ignores LastEvaluatedKey 404s here. + let scans = 0; + ddbSend.mockReset(); + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Scan' && cmd.input.TableName === 'LinearRegistry') { + scans += 1; + if (scans === 1) { + // Page 1: empty (no matching row) + a continuation key. An empty + // page plus a LastEvaluatedKey is exactly the shape that catches a + // non-paginating handler. + return Promise.resolve({ Items: [], LastEvaluatedKey: { _idx: 1 } }); + } + // Page 2: the target. + return Promise.resolve({ Items: [activeRow()] }); + } + return Promise.resolve({}); + }); + smSend.mockReset(); + smSend.mockResolvedValue({}); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + expect(scans).toBe(2); + // The second scan carried the continuation key from page one. + const secondScan = ddbSend.mock.calls.filter( + ([c]) => c._type === 'Scan' && c.input.TableName === 'LinearRegistry', + )[1]; + expect(secondScan![0].input.ExclusiveStartKey).toEqual({ _idx: 1 }); + // No `Limit` on a filtered registry scan (that was the B1 bug). + const firstScan = ddbSend.mock.calls.find( + ([c]) => c._type === 'Scan' && c.input.TableName === 'LinearRegistry', + ); + expect(firstScan![0].input.Limit).toBeUndefined(); + }); + + test('already-revoked workspace is treated as not-found (fail-closed, no re-revoke)', async () => { + // The registry scan filters on status='active', so an already-revoked + // row simply doesn't match — the router models that by returning no + // items for a non-active seed. The 404 does not distinguish revoked from + // missing, and avoids a second destructive pass. + routeDdb({ registryRow: activeRow({ status: 'revoked' }) }); + smSend.mockReset(); + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(404); + expect(smSend).not.toHaveBeenCalled(); + }); + + test('the registry scan fail-closes on status via the FilterExpression (pins the filter to the handler)', async () => { + // Assert the handler itself sends #status = :active — the revoke-oracle + // prevention property lives in this filter, not in the test router. + routeDdb({ registryRow: null }); + await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + const scanCall = ddbSend.mock.calls.find( + ([c]) => c._type === 'Scan' && c.input.TableName === 'LinearRegistry', + ); + expect(scanCall![0].input.FilterExpression).toContain('#status'); + expect(scanCall![0].input.ExpressionAttributeValues).toMatchObject({ ':active': 'active' }); + // Read-your-writes: a default eventually-consistent Scan can hand back a + // row a just-completed setup (or a peer removal) has already changed. + // Same reasoning as `shared/jira-tenant-registry.ts:32-46`. + expect(scanCall![0].input.ConsistentRead).toBe(true); + }); + + test('a real (non-idempotent) secret-delete error 500s SECRET_DELETE_FAILED and marks the row', async () => { + // The row is revoked first (fail-closed holds), but the live OAuth secret + // could not be deleted. This must NOT be masked as success, and must not + // be an opaque 500 — the operator needs to know a credential leaked. + routeDdb(); + smSend.mockReset(); + smSend.mockRejectedValueOnce(Object.assign(new Error('denied'), { name: 'AccessDeniedException' })); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(500); + const body = JSON.parse(result.body) as { error: { code: string } }; + expect(body.error.code).toBe('SECRET_DELETE_FAILED'); + + // The registry row was still revoked (Update ran before the secret step)... + const revokeUpdate = findRevoke(); + expect(revokeUpdate).toBeTruthy(); + // ...and a durable secret-deletion-failed marker was persisted, naming the + // exact SecretId that was attempted so the orphan is findable. + const marker = findMarker(); + expect(marker).toBeTruthy(); + expect(marker![0].input.ExpressionAttributeValues).toMatchObject({ + ':t': true, + ':e': 'AccessDeniedException', + ':arn': 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-linear-oauth-acme-AbCd', + }); + }); + + test('B3 regression: --purge + secret-delete failure keeps the row and marks it (no leaked credential)', async () => { + // On --purge the row is revoked first (an Update, NOT a delete), the + // secret delete fails, and the --purge row delete must NOT run — so the + // durable orphaned-secret marker survives and the credential is + // discoverable. Before the reorder fix the row was deleted up-front and + // the marker was skipped, leaking the secret with no record. + routeDdb(); + smSend.mockReset(); + smSend.mockRejectedValueOnce(Object.assign(new Error('denied'), { name: 'AccessDeniedException' })); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN, query: { purge: 'true' } })); + expect(result.statusCode).toBe(500); + const body = JSON.parse(result.body) as { error: { code: string } }; + expect(body.error.code).toBe('SECRET_DELETE_FAILED'); + + // The row was revoked (Update), the marker was persisted, and — crucially + // — no Delete ran, so the row (and its marker) survives on the --purge path. + expect(findRevoke()).toBeTruthy(); + expect(findMarker()).toBeTruthy(); + expect(ddbSend.mock.calls.filter(([c]) => c._type === 'Delete')).toHaveLength(0); + }); + + test('--purge deletes the row only AFTER the secret is confirmed gone (revoke → delete-secret → delete-row)', async () => { + routeDdb(); + smSend.mockReset(); + smSend.mockResolvedValue({}); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN, query: { purge: 'true' } })); + expect(result.statusCode).toBe(200); + + // Order: registry Update (revoke) → DeleteSecret → registry Delete (purge). + const ddbTypes = ddbSend.mock.calls.map(([c]) => c._type); + const updateIdx = ddbTypes.indexOf('Update'); + const deleteIdx = ddbTypes.indexOf('Delete'); + expect(updateIdx).toBeGreaterThanOrEqual(0); + expect(deleteIdx).toBeGreaterThan(updateIdx); + expect(smSend).toHaveBeenCalledTimes(1); + + const body = JSON.parse(result.body) as { data: { status: string } }; + expect(body.data.status).toBe('purged'); + }); + + // ─── B1: a row with no recorded ARN must still be torn down ───────────── + test('a registry row with no oauth_secret_arn deletes the secret by its deterministic name', async () => { + // The old handler skipped the secret delete entirely when the row had no + // `oauth_secret_arn`, silently leaving a live Linear OAuth secret behind + // and reporting success. The name is deterministic + // (`bgagent-linear-oauth-`) and `SecretId` accepts a name, so there + // is nothing to guess — and the IAM grant is over that *name* prefix + // (`linear-integration.ts:519`), which is what makes this permitted. + routeDdb({ registryRow: activeRow({ oauth_secret_arn: undefined }) }); + smSend.mockReset(); + smSend.mockResolvedValue({}); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + expect(smSend).toHaveBeenCalledTimes(1); + expect(smSend.mock.calls[0][0].input).toEqual({ + SecretId: 'bgagent-linear-oauth-acme', + ForceDeleteWithoutRecovery: true, + }); + const body = JSON.parse(result.body) as { data: { secret: string } }; + expect(body.data.secret).toBe('deleted'); + }); + + test("a vault-managed row with no secret reports secret: 'not_applicable' and echoes provider_name", async () => { + // The distinction the boolean erased. `absent` says "teardown finished"; + // `not_applicable` says "this workspace's credential lives in an AgentCore + // provider that this endpoint did not delete" — a live, self-refreshing + // Linear grant that outlives even `cdk destroy`. Same observable AWS calls, + // opposite operational meaning. + routeDdb({ + registryRow: activeRow({ + oauth_secret_arn: undefined, + provider_name: 'bgagent-linear-oauth-acme', + vault_user_id: 'vault-user-1', + }), + }); + smSend.mockReset(); + smSend.mockRejectedValueOnce( + Object.assign(new Error('not found'), { name: 'ResourceNotFoundException' }), + ); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + const body = JSON.parse(result.body) as { data: { secret: string; provider_name?: string } }; + expect(body.data.secret).toBe('not_applicable'); + expect(body.data.provider_name).toBe('bgagent-linear-oauth-acme'); + }); + + test("a vault row that DID record an ARN reports 'absent', not 'not_applicable'", async () => { + // `bgagent linear setup` writes `oauth_secret_arn` unconditionally + // (cli/src/commands/linear.ts:1321,1340), so vault rows normally carry BOTH + // a provider name and an ARN. "vault-managed ⇒ no secret" would therefore be + // wrong: only a vault row with no ARN of its own is `not_applicable`. The + // provider follow-up is still reported, because that is driven by + // `provider_name`, not by the secret outcome. + routeDdb({ registryRow: activeRow({ provider_name: 'bgagent-linear-oauth-acme' }) }); + smSend.mockReset(); + smSend.mockRejectedValueOnce( + Object.assign(new Error('not found'), { name: 'ResourceNotFoundException' }), + ); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + const body = JSON.parse(result.body) as { data: { secret: string; provider_name?: string } }; + expect(body.data.secret).toBe('absent'); + expect(body.data.provider_name).toBe('bgagent-linear-oauth-acme'); + }); + + // ─── Concurrency + scan bounds ────────────────────────────────────────── + test('a lost race (row no longer active at write time) 404s and never deletes the secret', async () => { + // The filtered Scan and the Update are two round trips, so a peer DELETE + // (or the resolver latching the row) can land in between. The + // ConditionExpression is what catches that; without it both callers would + // "succeed" and the second would delete a secret the first already + // accounted for. + routeDdb(); + const baseImpl = ddbSend.getMockImplementation()!; + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Update') { + return Promise.reject( + Object.assign(new Error('conditional check failed'), { name: 'ConditionalCheckFailedException' }), + ); + } + return baseImpl(cmd); + }); + smSend.mockReset(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(404); + const body = JSON.parse(result.body) as { error: { code: string } }; + expect(body.error.code).toBe('WORKSPACE_NOT_FOUND'); + // Nothing destructive may follow a lost race. + expect(smSend).not.toHaveBeenCalled(); + expect(ddbSend.mock.calls.filter(([c]) => c._type === 'Delete')).toHaveLength(0); + }); + + test('a non-conditional Update failure surfaces as a 500 and never reaches the secret delete', async () => { + // Only ConditionalCheckFailedException means "someone else won". Any other + // Update error (throttle, IAM, table gone) must NOT be swallowed into a + // 404, and must not let the secret delete run against a row that is still + // active — that would leave a workspace whose token resolves but whose + // credential is gone. + routeDdb(); + const baseImpl = ddbSend.getMockImplementation()!; + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Update') { + return Promise.reject(Object.assign(new Error('throttled'), { name: 'ProvisionedThroughputExceededException' })); + } + return baseImpl(cmd); + }); + smSend.mockReset(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(500); + expect(smSend).not.toHaveBeenCalled(); + }); + + test('the registry scan is bounded: a never-matching paginating scan 500s instead of burning the timeout', async () => { + // An unbounded `do { ... } while (!row && key)` on a large registry can + // spend the whole 10s Lambda budget and die with an opaque timeout. A + // clean 500 that names the cap is diagnosable; a timeout is not. + let scans = 0; + ddbSend.mockReset(); + ddbSend.mockImplementation((cmd: { _type: string }) => { + if (cmd._type === 'Scan') { + scans += 1; + // Always empty, always another page. + return Promise.resolve({ Items: [], LastEvaluatedKey: { _idx: scans } }); + } + return Promise.resolve({}); + }); + smSend.mockReset(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(500); + // Bounded, and the bound is the handler's MAX_SCAN_PAGES (20). + expect(scans).toBe(20); + expect(smSend).not.toHaveBeenCalled(); + }); + + test('a --purge row delete failure after a successful secret delete 500s (does not report purged)', async () => { + // The secret is already destroyed at this point, so the workspace cannot + // authenticate — but the row is still there as `revoked`. Reporting 200 + // `purged` would claim a row deletion that never happened. + routeDdb(); + const baseImpl = ddbSend.getMockImplementation()!; + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Delete') { + return Promise.reject(Object.assign(new Error('denied'), { name: 'AccessDeniedException' })); + } + return baseImpl(cmd); + }); + smSend.mockReset(); + smSend.mockResolvedValue({}); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN, query: { purge: 'true' } })); + expect(result.statusCode).toBe(500); + // The revoke still landed, so the workspace is fail-closed regardless. + expect(findRevoke()).toBeTruthy(); + expect(smSend).toHaveBeenCalledTimes(1); + }); + + test('a failing marker write does not mask the SECRET_DELETE_FAILED error', async () => { + // The marker is best-effort telemetry; if persisting it also fails, the + // caller must still get the loud, specific error rather than an opaque + // 500 from the marker's own rejection. + routeDdb(); + const baseImpl = ddbSend.getMockImplementation()!; + ddbSend.mockImplementation((cmd: { _type: string; input: Record }) => { + if (cmd._type === 'Update' && String(cmd.input.UpdateExpression ?? '').includes('secret_deletion_failed')) { + return Promise.reject(new Error('marker write failed')); + } + return baseImpl(cmd); + }); + smSend.mockReset(); + smSend.mockRejectedValueOnce(Object.assign(new Error('denied'), { name: 'AccessDeniedException' })); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(500); + const body = JSON.parse(result.body) as { error: { code: string } }; + expect(body.error.code).toBe('SECRET_DELETE_FAILED'); + }); +}); + +describe('linear-remove-workspace handler without its registry table configured', () => { + test('500s on a missing LINEAR_WORKSPACE_REGISTRY_TABLE_NAME instead of an opaque SDK error', async () => { + // The env var is read at module scope as `string | undefined` (matching + // every other reader of it) and guarded once inside the handler. A `!` + // would assert away a deploy misconfiguration and surface it as + // `TableName: undefined` from the SDK, several frames from the cause. + // Re-imported in isolation because the read happens at module load. + const saved = process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME; + delete process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME; + try { + jest.resetModules(); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const mod = require('../../src/handlers/linear-remove-workspace') as typeof import('../../src/handlers/linear-remove-workspace'); + ddbSend.mockReset(); + smSend.mockReset(); + const result = await mod.handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(500); + // It fails before any AWS call — no scan against an undefined table. + expect(ddbSend).not.toHaveBeenCalled(); + expect(smSend).not.toHaveBeenCalled(); + } finally { + process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = saved; + jest.resetModules(); + } + }); +}); diff --git a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts index dcc16c416..73b1d16dc 100644 --- a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts +++ b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts @@ -310,6 +310,42 @@ describe('resolveLinearOauthToken', () => { expect(result).toBeNull(); }); + // Adversarial fail-closed guard for `bgagent linear remove-workspace` (#306): + // after a workspace is revoked (registry row flipped to status='revoked'), + // a request for that slug MUST NOT resolve to a usable token — even if a + // perfectly valid, non-expiring OAuth secret is still sitting in Secrets + // Manager. The status gate short-circuits BEFORE the secret is ever read, + // so a revoked workspace can never leak its token. + test('fail-closed: a revoked workspace is rejected without ever reading the secret', async () => { + const clients = makeFakeClients({ + registryItem: { + workspace_slug: 'acme', + oauth_secret_arn: 'arn:secret:acme', + status: 'revoked', + }, + // A fully valid, far-future token — the ONLY thing that should block + // resolution here is the revoked status. + storedToken: makeStoredToken({ access_token: 'lin_oauth_still_valid' }), + }); + const result = await resolveLinearOauthToken('ws-uuid-1', REGISTRY_TABLE, clients); + expect(result).toBeNull(); + // The secret must never be fetched for a revoked workspace. + expect(clients.smSend).not.toHaveBeenCalled(); + }); + + test('control: the same secret DOES resolve when the workspace is active', async () => { + const clients = makeFakeClients({ + registryItem: { + workspace_slug: 'acme', + oauth_secret_arn: 'arn:secret:acme', + status: 'active', + }, + storedToken: makeStoredToken({ access_token: 'lin_oauth_still_valid' }), + }); + const result = await resolveLinearOauthToken('ws-uuid-1', REGISTRY_TABLE, clients); + expect(result?.accessToken).toBe('lin_oauth_still_valid'); + }); + test('returns null when secret JSON is missing required fields', async () => { const clients = makeFakeClients({ registryItem: { diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 88d48bc3c..4ca6d4428 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -1608,7 +1608,8 @@ describe('AgentStack solution attribution (#319): AWS_SDK_UA_APP_ID via stack-le // A loose `toBeGreaterThan` let a whole integration construct disappear // unnoticed; the exact count fails if a Lambda is dropped OR if a new one // is added without being attributed below. - expect(abcaLambdas.length).toBe(46); + // 47 = 46 on main + RemoveWorkspaceFn (DELETE /v1/linear/workspaces/{slug}). + expect(abcaLambdas.length).toBe(47); // Every ABCA-authored Lambda must carry the canonical `#` app-id. Collect // any offenders so a failure names the exact logical id(s) that are naked. const unattributed = abcaLambdas diff --git a/cli/src/api-client.ts b/cli/src/api-client.ts index 678408120..5af425ab8 100644 --- a/cli/src/api-client.ts +++ b/cli/src/api-client.ts @@ -40,6 +40,7 @@ import { GetPoliciesResponse, JiraLinkResponse, LinearLinkResponse, + LinearRemoveWorkspaceResponse, NudgeRequest, NudgeResponse, RegistryListEntry, @@ -535,6 +536,24 @@ export class ApiClient { return res.data; } + /** DELETE /linear/workspaces/{slug} — deregister a Linear workspace. + * + * Server-side: revokes the registry row (or deletes it with `purge`) and + * deletes the per-workspace OAuth secret. Admin-only, enforced by the + * handler against the recorded installer identity. Project→repo mappings + * are not touched (they carry no workspace id — remove by project id). */ + async linearRemoveWorkspace( + slug: string, + opts: { purge?: boolean } = {}, + ): Promise { + const params = new URLSearchParams(); + if (opts.purge) params.set('purge', 'true'); + const qs = params.toString(); + const path = `/linear/workspaces/${encodeURIComponent(slug)}${qs ? `?${qs}` : ''}`; + const res = await this.request>('DELETE', path); + return res.data; + } + /** POST /jira/link — link a Jira account using a verification code. * * `dryRun: true` returns the identity attached to the code without diff --git a/cli/src/commands/linear.ts b/cli/src/commands/linear.ts index ced1b38a6..482aec1d0 100644 --- a/cli/src/commands/linear.ts +++ b/cli/src/commands/linear.ts @@ -1953,6 +1953,108 @@ export function makeLinearCommand(): Command { }), ); + linear.addCommand( + new Command('remove-workspace') + .description('Deregister a Linear workspace: revoke the registry row + delete its OAuth secret') + .argument('', 'Linear workspace urlKey (e.g. "acme" from linear.app/acme/...)') + .option('--purge', 'Delete the registry row entirely instead of keeping it with status=revoked (no audit trail)') + .option('--yes', 'Skip the slug-confirmation prompt (for scripted use)') + .action(async (slug: string, opts) => { + // Undoes `bgagent linear setup` / `add-workspace`. The destructive + // work (registry revoke, Secrets Manager delete) happens server-side + // behind a DELETE call so DDB / Secrets Manager grants stay on the + // API role, not on every CLI user's IAM identity. This mirrors + // `link`, which also delegates its writes to the backend rather than + // touching AWS directly. + // + // By default this is a SOFT removal: the registry row is flipped to + // status=revoked with revoked_reason=admin_removed (preserving the + // audit trail), so the workspace can no longer resolve a token or + // route webhooks the instant this returns. The resolver refuses every + // non-active row *except* one it re-probes — a `revoked` row whose + // reason is `vault_consent_required` — and `admin_removed` is + // deliberately not that reason, which is what makes an admin removal + // terminal rather than a latch a later vault probe can clear. + // + // Project→repo mappings are NOT touched: mapping rows carry no + // workspace id, so they can't be attributed to a workspace. Remove + // a mapping by project id (see LINEAR_SETUP_GUIDE). + if (!SLUG_RE.test(slug)) { + throw new CliError( + `Invalid workspace slug '${slug}'. Must be 4-50 chars matching [a-zA-Z0-9_-]. ` + + 'This is the Linear urlKey, e.g. \'acme\' from linear.app/acme/...', + ); + } + + const purge = Boolean(opts.purge); + + // Slug-confirmation prompt (skipped by --yes). Typing the slug is a + // deliberate speed-bump before an irreversible teardown — the same + // "type the name to confirm" pattern used by destructive CLIs. + if (!opts.yes) { + console.log(`About to remove Linear workspace '${slug}'. This will:`); + console.log(purge + ? ' • DELETE the registry row entirely (no audit trail)' + : ' • Mark the registry row status=revoked (preserves audit trail)'); + console.log(` • Delete the Secrets Manager secret '${linearOauthSecretName(slug)}' if it exists`); + console.log(' • Leave project→repo mappings in place (remove those by project id)'); + // Whether this workspace is vault-managed is only known server-side + // (it is a registry-row attribute), so the follow-up command can't be + // named until the response comes back — hence the warning here and + // the exact command after. + console.log(' • NOT delete an AgentCore credential provider, if this workspace is vault-managed'); + console.log(); + const confirm = (await promptLine('Type the workspace slug to confirm')).trim(); + if (confirm !== slug) { + console.log('Aborted — the confirmation did not match the slug. Nothing was removed.'); + return; + } + } + + const client = new ApiClient(); + const result = await client.linearRemoveWorkspace(slug, { purge }); + + console.log(); + console.log(`✅ Workspace '${result.workspace_slug}' removed (${result.status}).`); + console.log(result.status === 'purged' + ? ' ✓ Registry row deleted' + : ' ✓ Registry row revoked'); + // Three states, not a boolean: `absent` and `not_applicable` both mean + // "nothing was deleted here", but only `absent` means teardown is + // finished. A vault-managed workspace keeps its credential in an + // AgentCore OAuth2 credential provider that lives outside + // CloudFormation — `cdk destroy` will not remove it and it never shows + // up in `cdk diff`/drift — so leaving that as a silent "nothing to + // delete" strands a live, self-refreshing Linear grant. + switch (result.secret) { + case 'deleted': + console.log(' ✓ OAuth secret deleted'); + break; + case 'absent': + console.log(' • OAuth secret was already absent (nothing to delete)'); + break; + case 'not_applicable': + console.log(' • No Secrets Manager secret for this workspace — its credential is vault-managed'); + break; + } + console.log(' • Project→repo mappings left in place — remove by project id if needed'); + + if (result.provider_name) { + // Printed verbatim from the response rather than re-derived from the + // slug: the provider name is chosen at onboarding time and the docs + // are not a reliable source for its prefix. Echoing the server's + // value keeps this command copy-pasteable even if that naming + // changes. + console.log(); + console.log('⚠️ Teardown is NOT complete. This workspace is vault-managed, and its'); + console.log(' AgentCore credential provider still holds the Linear client secret and a'); + console.log(' live refresh grant. It is not managed by CloudFormation — delete it with:'); + console.log(); + console.log(` aws bedrock-agentcore-control delete-oauth2-credential-provider --name ${result.provider_name}`); + } + }), + ); + linear.addCommand( new Command('invite-user') .description('Generate a one-time code for a Linear teammate to redeem via `bgagent linear link `') diff --git a/cli/src/types.ts b/cli/src/types.ts index 7655fea7c..e1b6b681a 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -625,6 +625,45 @@ export interface LinearLinkResponse { readonly linked_at?: string; } +/** Linear remove-workspace response from DELETE /v1/linear/workspaces/{slug}. + * + * `status` is `revoked` for the default soft-removal (registry row kept with + * `status=revoked` and `revoked_reason=admin_removed` for audit) or `purged` + * when the row was deleted outright (`--purge`). A revoked row no longer + * resolves tokens: the OAuth resolver refuses any non-active row, and + * `admin_removed` is deliberately not the one reason it re-probes instead of + * refusing — so the removal is terminal, not a latch a later vault probe can + * clear. Project→repo mappings are not touched (they carry no workspace id and + * are removed by project id). + * + * `provider_name` is present only for vault-managed workspaces, and its + * presence means **teardown is not finished**: an AgentCore OAuth2 credential + * provider survives outside CloudFormation, still holding the Linear client + * secret and a live, self-refreshing grant. The CLI prints the + * `delete-oauth2-credential-provider` follow-up when it is set. */ +export interface LinearRemoveWorkspaceResponse { + readonly workspace_slug: string; + readonly linear_workspace_id: string; + readonly status: 'revoked' | 'purged'; + /** + * What happened to the per-workspace `bgagent-linear-oauth-` secret. + * Three states, not a boolean, because "nothing was deleted" does not always + * mean "teardown is complete": + * - `deleted` — a live secret was destroyed by this call. + * - `absent` — there was supposed to be one and it is already gone (a re-run, + * or a prior partial teardown). Idempotent success. + * - `not_applicable` — the workspace is vault-managed and never had a Secrets + * Manager secret of its own; its credential lives in the AgentCore provider + * named by `provider_name`, which this endpoint does **not** delete. + * + * Inlined rather than given its own exported alias: `check-types-sync.ts` + * treats every exported CLI type as drift unless CDK exports it too, and the + * handler's matching shape is a module-local interface there. + */ + readonly secret: 'deleted' | 'absent' | 'not_applicable'; + readonly provider_name?: string; +} + /** Jira link response from POST /v1/jira/link. * * Mirrors LinearLinkResponse semantics: `dry_run: true` returns the diff --git a/cli/test/api-client.test.ts b/cli/test/api-client.test.ts index d0189df86..5861e809e 100644 --- a/cli/test/api-client.test.ts +++ b/cli/test/api-client.test.ts @@ -190,6 +190,42 @@ describe('ApiClient', () => { }); }); + describe('linearRemoveWorkspace', () => { + const okBody = { + ok: true, + json: async () => ({ + data: { + workspace_slug: 'acme', + linear_workspace_id: 'ws-1', + status: 'revoked', + secret: 'deleted', + }, + }), + }; + + test('sends DELETE to /linear/workspaces/{slug} with no query string by default', async () => { + mockFetch.mockResolvedValue(okBody); + await client.linearRemoveWorkspace('acme'); + expect(mockFetch).toHaveBeenCalledWith( + 'https://api.example.com/linear/workspaces/acme', + expect.objectContaining({ method: 'DELETE' }), + ); + }); + + test('maps --purge to the snake_case query param (matches handler reads)', async () => { + mockFetch.mockResolvedValue(okBody); + await client.linearRemoveWorkspace('acme', { purge: true }); + const url = mockFetch.mock.calls[0][0] as string; + expect(url).toContain('purge=true'); + }); + + test('URL-encodes the slug', async () => { + mockFetch.mockResolvedValue(okBody); + await client.linearRemoveWorkspace('a b'); + expect(mockFetch.mock.calls[0][0]).toContain('/linear/workspaces/a%20b'); + }); + }); + describe('getTaskEvents', () => { test('sends GET to events endpoint', async () => { const response = { data: [], pagination: { next_token: null, has_more: false } }; diff --git a/cli/test/commands/linear-remove-workspace.test.ts b/cli/test/commands/linear-remove-workspace.test.ts new file mode 100644 index 000000000..fa5092beb --- /dev/null +++ b/cli/test/commands/linear-remove-workspace.test.ts @@ -0,0 +1,212 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { ApiClient } from '../../src/api-client'; +import { makeLinearCommand } from '../../src/commands/linear'; +import { CliError } from '../../src/errors'; + +jest.mock('../../src/api-client'); + +const mockRemove = jest.fn(); + +function installMockClient() { + (ApiClient as jest.MockedClass).mockImplementation(() => ({ + linearRemoveWorkspace: mockRemove, + }) as unknown as ApiClient); +} + +/** Run `bgagent linear remove-workspace ...`. */ +async function runRemove(args: string[]): Promise { + const cmd = makeLinearCommand(); + await cmd.parseAsync(['node', 'test', 'remove-workspace', ...args]); +} + +describe('linear remove-workspace command', () => { + let logSpy: jest.SpiedFunction; + + beforeEach(() => { + mockRemove.mockReset(); + installMockClient(); + logSpy = jest.spyOn(console, 'log').mockImplementation(); + }); + + afterEach(() => { + logSpy.mockRestore(); + }); + + test('--yes skips the prompt and calls DELETE with default flags', async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'deleted', + }); + + await runRemove(['acme', '--yes']); + + expect(mockRemove).toHaveBeenCalledTimes(1); + expect(mockRemove).toHaveBeenCalledWith('acme', { purge: false }); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('revoked'); + }); + + test('--purge forwards purge=true to the API', async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'purged', + secret: 'deleted', + }); + + await runRemove(['acme', '--yes', '--purge']); + + expect(mockRemove).toHaveBeenCalledWith('acme', { purge: true }); + }); + + test('rejects an invalid slug without hitting the API', async () => { + await expect(runRemove(['a', '--yes'])).rejects.toBeInstanceOf(CliError); + expect(mockRemove).not.toHaveBeenCalled(); + }); + + test('surfaces the API error (does not swallow)', async () => { + mockRemove.mockRejectedValue(new CliError('Workspace not found.')); + await expect(runRemove(['ghost', '--yes'])).rejects.toThrow('Workspace not found.'); + }); + + test('does not claim any project-mapping cleanup in the success output', async () => { + // Mapping cleanup was dropped (rows carry no workspace id); the command + // must not report a mapping count or a checkmark implying it ran. + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'deleted', + }); + + await runRemove(['acme', '--yes']); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).not.toContain('mapping(s) removed'); + expect(out).toContain('mappings left in place'); + }); + + test("reports when the OAuth secret was already absent (secret: 'absent')", async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'absent', + }); + + await runRemove(['acme', '--yes']); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('already absent'); + // `absent` means teardown IS finished — no vault follow-up must appear. + expect(out).not.toContain('delete-oauth2-credential-provider'); + }); + + // ─── Vault-managed teardown is incomplete (the B1 bug) ────────────────── + // `secret: 'absent'` and `secret: 'not_applicable'` both mean "no secret was + // deleted", but only the first means the workspace is fully torn down. When a + // provider name comes back, an AgentCore credential provider outside + // CloudFormation still holds the Linear client secret and a live refresh + // grant, and the operator has to delete it by hand. Collapsing the two into + // one boolean is what hid that. + test('prints the AgentCore follow-up command for a vault-managed workspace', async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'not_applicable', + provider_name: 'bgagent-linear-oauth-acme', + }); + + await runRemove(['acme', '--yes']); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('vault-managed'); + expect(out).toContain('Teardown is NOT complete'); + expect(out).toContain( + 'aws bedrock-agentcore-control delete-oauth2-credential-provider --name bgagent-linear-oauth-acme', + ); + }); + + test('echoes the returned provider name verbatim rather than deriving it from the slug', async () => { + // The provider name is minted at onboarding and the response is the only + // authority on it — a CLI that rebuilt `` would print a + // command that silently no-ops if the convention ever changes. + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'deleted', + provider_name: 'legacy-linear-provider-acme-7f3a', + }); + + await runRemove(['acme', '--yes']); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('--name legacy-linear-provider-acme-7f3a'); + expect(out).not.toContain('--name bgagent-linear-oauth-acme'); + // A vault row can also have had its own secret deleted; the follow-up is + // driven by `provider_name`, not by the secret outcome. + expect(out).toContain('✓ OAuth secret deleted'); + }); + + // ─── Confirmation prompt (the destructive-command safety rail) ────────── + // Without --yes the command reads a slug via promptLine and must abort on + // mismatch. Under Jest, promptLine takes the non-TTY readline branch. + function mockPromptLine(returned: string) { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const readline = require('readline') as typeof import('readline'); + const rlMock = { + once: (event: string, cb: (line: string) => void) => { + if (event === 'line') cb(returned); + }, + close: jest.fn(), + }; + return jest.spyOn(readline, 'createInterface') + .mockReturnValue(rlMock as unknown as ReturnType); + } + + test('aborts without calling the API when the typed confirmation does not match the slug', async () => { + const rlSpy = mockPromptLine('wrong-slug'); + try { + await runRemove(['acme']); + expect(mockRemove).not.toHaveBeenCalled(); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('Aborted'); + } finally { + rlSpy.mockRestore(); + } + }); + + test('proceeds when the typed confirmation matches the slug', async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret: 'deleted', + }); + const rlSpy = mockPromptLine('acme'); + try { + await runRemove(['acme']); + expect(mockRemove).toHaveBeenCalledWith('acme', { purge: false }); + } finally { + rlSpy.mockRestore(); + } + }); +}); diff --git a/docs/guides/LINEAR_SETUP_GUIDE.md b/docs/guides/LINEAR_SETUP_GUIDE.md index 8c95b9449..e4b6685b9 100644 --- a/docs/guides/LINEAR_SETUP_GUIDE.md +++ b/docs/guides/LINEAR_SETUP_GUIDE.md @@ -307,9 +307,35 @@ A `WARN … Ignoring Linear agent-mode webhook …` line names the offending wor - `resolve_linear_api_token: invalid_grant` — Linear permanently rejected the refresh token. Re-run setup to issue a new one. - On the vault path, a "consent required" outcome means the grant is gone; re-run setup. -## Removing the integration +## Removing a workspace -Deactivate a project mapping: +Deregister a workspace with a single command — the inverse of `setup` / `add-workspace`: + +```bash +bgagent linear remove-workspace +``` + +This runs server-side (through an authenticated `DELETE /v1/linear/workspaces/{slug}` call, so the DynamoDB and Secrets Manager permissions stay on the API role, not on your local IAM identity) and by default: + +- Marks the registry row `status=revoked` with `revoked_reason=admin_removed` (preserves the audit trail), so the workspace stops resolving tokens and routing webhooks the instant the command returns. The OAuth resolver refuses every non-`active` row except one it re-probes — a `revoked` row whose reason is `vault_consent_required` — and `admin_removed` is deliberately not that reason, so an admin removal is terminal rather than a latch a later vault probe can clear. +- Deletes the per-workspace `bgagent-linear-oauth-` secret from Secrets Manager, and reports which of three things happened: `deleted` (a live secret was destroyed), `absent` (already gone — a re-run, or a prior partial teardown), or `not_applicable` (this workspace is vault-managed and has no Secrets Manager secret of its own). + +Only the workspace **admin** — the platform user who ran `setup` / `add-workspace` for the slug — may remove it. You are prompted to re-type the slug before anything is torn down. + +> **Vault-managed workspaces are not fully torn down by this command.** If the registry row has a `provider_name`, the workspace's credential lives in an AgentCore OAuth2 credential provider created outside CloudFormation, and this command does not delete it — it names it. The CLI prints the exact `delete-oauth2-credential-provider` follow-up; see [Vault-managed workspaces: the credential provider outlives the stack](#vault-managed-workspaces-the-credential-provider-outlives-the-stack) below. + +Flags: + +- `--purge` — delete the registry row entirely instead of keeping it with `status=revoked` (drops the audit trail). The row is still revoked first (fail-closed) and is only hard-deleted after the OAuth secret is confirmed gone. +- `--yes` — skip the slug-confirmation prompt (for scripted use). + +> **Project→repo mappings are not removed by this command.** Mapping rows carry no workspace identifier, so they cannot be attributed to a workspace. Remove a workspace's mappings by project id (see below). + +Then delete the Linear webhook from [Linear Settings → API](https://linear.app/settings/api) and uninstall the OAuth app from [Workspace Settings → Integrations](https://linear.app/settings/integrations) on the Linear side. + +### Deactivating a single project mapping + +To remove one project→repo mapping (the only supported way to tear a mapping down): ```bash aws dynamodb update-item \ @@ -320,20 +346,27 @@ aws dynamodb update-item \ --expression-attribute-values '{":removed":{"S":"removed"}}' ``` -Revoke a workspace install: +### Manual fallback -```bash -aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery +If `remove-workspace` returns the `SECRET_DELETE_FAILED` error code, the workspace is already revoked (fail-closed, so it no longer resolves tokens or routes webhooks), but its OAuth secret was orphaned. The handler records this durably on the registry row: `secret_deletion_failed = true`, `secret_deletion_error` (the failing error name), and `orphaned_oauth_secret_arn` (the exact secret ARN to purge). When you see that marker — or the error code — run the `delete-secret` step below against `orphaned_oauth_secret_arn` to finish teardown. +If the CLI is unavailable, you can revoke a workspace directly (equivalent to the default `remove-workspace` flow). **Run the registry revoke first, then delete the secret** — the same order the handler uses. Deleting the secret first leaves an `active` registry row with no credential behind it: token resolution then fails per-event without latching the row (the resolver deliberately declines to infer a revocation from a missing secret), so the workspace keeps advertising itself as active, and webhook signature verification — which lets Secrets Manager errors bubble so a transient failure can't silently downgrade to the stack-wide secret — makes the receiver return 500 and Linear retry the delivery. Revoking first shuts the workspace off deterministically at the registry, and the secret delete is then pure cleanup. + +```bash aws dynamodb update-item \ --table-name \ --key '{"linear_workspace_id":{"S":""}}' \ - --update-expression 'SET #s = :revoked' \ + --update-expression 'SET #s = :revoked, revoked_reason = :reason' \ + --condition-expression '#s = :active' \ --expression-attribute-names '{"#s":"status"}' \ - --expression-attribute-values '{":revoked":{"S":"revoked"}}' + --expression-attribute-values '{":revoked":{"S":"revoked"},":active":{"S":"active"},":reason":{"S":"admin_removed"}}' + +aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery ``` -Then delete the webhook from [Linear Settings → API](https://linear.app/settings/api) and uninstall the app from [Workspace Settings → Integrations](https://linear.app/settings/integrations). +The `--condition-expression` mirrors the handler's, so a second run against an already-revoked row fails with `ConditionalCheckFailedException` instead of silently re-revoking. `remove-workspace` behaves the same way: because it only matches `status='active'` rows, **re-running it on an already-removed workspace returns 404**, not a second success. That 404 does not distinguish "revoked" from "never existed". + +Vault-managed workspaces need one more step after these two — see [the credential-provider section](#vault-managed-workspaces-the-credential-provider-outlives-the-stack). ### Vault-managed workspaces: the credential provider outlives the stack diff --git a/docs/src/content/docs/using/Linear-setup-guide.md b/docs/src/content/docs/using/Linear-setup-guide.md index d2302413e..6460bf507 100644 --- a/docs/src/content/docs/using/Linear-setup-guide.md +++ b/docs/src/content/docs/using/Linear-setup-guide.md @@ -311,9 +311,35 @@ A `WARN … Ignoring Linear agent-mode webhook …` line names the offending wor - `resolve_linear_api_token: invalid_grant` — Linear permanently rejected the refresh token. Re-run setup to issue a new one. - On the vault path, a "consent required" outcome means the grant is gone; re-run setup. -## Removing the integration +## Removing a workspace -Deactivate a project mapping: +Deregister a workspace with a single command — the inverse of `setup` / `add-workspace`: + +```bash +bgagent linear remove-workspace +``` + +This runs server-side (through an authenticated `DELETE /v1/linear/workspaces/{slug}` call, so the DynamoDB and Secrets Manager permissions stay on the API role, not on your local IAM identity) and by default: + +- Marks the registry row `status=revoked` with `revoked_reason=admin_removed` (preserves the audit trail), so the workspace stops resolving tokens and routing webhooks the instant the command returns. The OAuth resolver refuses every non-`active` row except one it re-probes — a `revoked` row whose reason is `vault_consent_required` — and `admin_removed` is deliberately not that reason, so an admin removal is terminal rather than a latch a later vault probe can clear. +- Deletes the per-workspace `bgagent-linear-oauth-` secret from Secrets Manager, and reports which of three things happened: `deleted` (a live secret was destroyed), `absent` (already gone — a re-run, or a prior partial teardown), or `not_applicable` (this workspace is vault-managed and has no Secrets Manager secret of its own). + +Only the workspace **admin** — the platform user who ran `setup` / `add-workspace` for the slug — may remove it. You are prompted to re-type the slug before anything is torn down. + +> **Vault-managed workspaces are not fully torn down by this command.** If the registry row has a `provider_name`, the workspace's credential lives in an AgentCore OAuth2 credential provider created outside CloudFormation, and this command does not delete it — it names it. The CLI prints the exact `delete-oauth2-credential-provider` follow-up; see [Vault-managed workspaces: the credential provider outlives the stack](#vault-managed-workspaces-the-credential-provider-outlives-the-stack) below. + +Flags: + +- `--purge` — delete the registry row entirely instead of keeping it with `status=revoked` (drops the audit trail). The row is still revoked first (fail-closed) and is only hard-deleted after the OAuth secret is confirmed gone. +- `--yes` — skip the slug-confirmation prompt (for scripted use). + +> **Project→repo mappings are not removed by this command.** Mapping rows carry no workspace identifier, so they cannot be attributed to a workspace. Remove a workspace's mappings by project id (see below). + +Then delete the Linear webhook from [Linear Settings → API](https://linear.app/settings/api) and uninstall the OAuth app from [Workspace Settings → Integrations](https://linear.app/settings/integrations) on the Linear side. + +### Deactivating a single project mapping + +To remove one project→repo mapping (the only supported way to tear a mapping down): ```bash aws dynamodb update-item \ @@ -324,20 +350,27 @@ aws dynamodb update-item \ --expression-attribute-values '{":removed":{"S":"removed"}}' ``` -Revoke a workspace install: +### Manual fallback -```bash -aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery +If `remove-workspace` returns the `SECRET_DELETE_FAILED` error code, the workspace is already revoked (fail-closed, so it no longer resolves tokens or routes webhooks), but its OAuth secret was orphaned. The handler records this durably on the registry row: `secret_deletion_failed = true`, `secret_deletion_error` (the failing error name), and `orphaned_oauth_secret_arn` (the exact secret ARN to purge). When you see that marker — or the error code — run the `delete-secret` step below against `orphaned_oauth_secret_arn` to finish teardown. +If the CLI is unavailable, you can revoke a workspace directly (equivalent to the default `remove-workspace` flow). **Run the registry revoke first, then delete the secret** — the same order the handler uses. Deleting the secret first leaves an `active` registry row with no credential behind it: token resolution then fails per-event without latching the row (the resolver deliberately declines to infer a revocation from a missing secret), so the workspace keeps advertising itself as active, and webhook signature verification — which lets Secrets Manager errors bubble so a transient failure can't silently downgrade to the stack-wide secret — makes the receiver return 500 and Linear retry the delivery. Revoking first shuts the workspace off deterministically at the registry, and the secret delete is then pure cleanup. + +```bash aws dynamodb update-item \ --table-name \ --key '{"linear_workspace_id":{"S":""}}' \ - --update-expression 'SET #s = :revoked' \ + --update-expression 'SET #s = :revoked, revoked_reason = :reason' \ + --condition-expression '#s = :active' \ --expression-attribute-names '{"#s":"status"}' \ - --expression-attribute-values '{":revoked":{"S":"revoked"}}' + --expression-attribute-values '{":revoked":{"S":"revoked"},":active":{"S":"active"},":reason":{"S":"admin_removed"}}' + +aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery ``` -Then delete the webhook from [Linear Settings → API](https://linear.app/settings/api) and uninstall the app from [Workspace Settings → Integrations](https://linear.app/settings/integrations). +The `--condition-expression` mirrors the handler's, so a second run against an already-revoked row fails with `ConditionalCheckFailedException` instead of silently re-revoking. `remove-workspace` behaves the same way: because it only matches `status='active'` rows, **re-running it on an already-removed workspace returns 404**, not a second success. That 404 does not distinguish "revoked" from "never existed". + +Vault-managed workspaces need one more step after these two — see [the credential-provider section](#vault-managed-workspaces-the-credential-provider-outlives-the-stack). ### Vault-managed workspaces: the credential provider outlives the stack diff --git a/scripts/check-types-sync.ts b/scripts/check-types-sync.ts index bf2f2555b..162590276 100644 --- a/scripts/check-types-sync.ts +++ b/scripts/check-types-sync.ts @@ -144,6 +144,7 @@ const CLI_ONLY_ALLOWLIST = new Set([ 'CancelTaskResponse', 'SlackLinkResponse', 'LinearLinkResponse', + 'LinearRemoveWorkspaceResponse', 'JiraLinkResponse', 'TraceUrlResponse', // Error classification — derived server-side via a function and