diff --git a/cdk/src/constructs/linear-integration.ts b/cdk/src/constructs/linear-integration.ts index d51e043b5..83f6dcf4e 100644 --- a/cdk/src/constructs/linear-integration.ts +++ b/cdk/src/constructs/linear-integration.ts @@ -41,6 +41,13 @@ const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 30; /** 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. */ @@ -302,6 +309,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 // ═══════════════════════════════════════════════════════════════════════════ @@ -324,6 +367,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), + cognitoAuthOptions, + ); + // ═══════════════════════════════════════════════════════════════════════════ // cdk-nag suppressions // ═══════════════════════════════════════════════════════════════════════════ @@ -346,7 +398,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..f18d27450 --- /dev/null +++ b/cdk/src/handlers/linear-remove-workspace.ts @@ -0,0 +1,288 @@ +/** + * 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 { DynamoDBClient } from '@aws-sdk/client-dynamodb'; +import { DeleteSecretCommand, SecretsManagerClient } from '@aws-sdk/client-secrets-manager'; +import { DynamoDBDocumentClient, 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 { logger } from './shared/logger'; +import { ErrorCode, errorResponse, successResponse } from './shared/response'; + +const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({})); +const sm = new SecretsManagerClient({}); + +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}$/; + +/** + * 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'` (the OAuth resolver + * fail-closes on any status != 'active' — see + * `shared/linear-oauth-resolver.ts`, so a revoked workspace can no + * longer resolve a token and its inbound webhooks stop routing). + * 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: if the secret is already gone we report + * `secret_deleted: false` and still complete the revoke, so a retried or + * partially-completed removal converges cleanly. + * + * 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'; + + // ─── 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 is not a revoke-oracle and we never re-run the + // destructive path on a row that's already been torn down. + // + // 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 paginate to completion + // instead, matching the small-table convention already used for this + // registry in `jira-webhook-processor.ts` and `shared/linear-issue-lookup.ts`. + // 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. + let row: Record | undefined; + let scanKey: Record | undefined; + do { + const page = await ddb.send(new ScanCommand({ + TableName: WORKSPACE_REGISTRY_TABLE, + FilterExpression: 'workspace_slug = :slug AND #status = :active', + ExpressionAttributeNames: { '#status': 'status' }, + ExpressionAttributeValues: { ':slug': slug, ':active': 'active' }, + ExclusiveStartKey: scanKey, + })); + row = page.Items?.[0]; + scanKey = page.LastEvaluatedKey as Record | undefined; + } 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; + 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 fail-closes on any non-active status, + // so the workspace stops resolving tokens and routing webhooks the + // instant this write lands. 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. + await ddb.send(new UpdateCommand({ + TableName: WORKSPACE_REGISTRY_TABLE, + Key: { linear_workspace_id: linearWorkspaceId }, + UpdateExpression: 'SET #status = :revoked, revoked_at = :now, revoked_by_platform_user_id = :uid, updated_at = :now', + ExpressionAttributeNames: { '#status': 'status' }, + ExpressionAttributeValues: { ':revoked': 'revoked', ':now': now, ':uid': userId }, + })); + + // ─── 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. + phase = 'secret_delete'; + let secretDeleted = false; + if (oauthSecretArn) { + try { + await sm.send(new DeleteSecretCommand({ + SecretId: oauthSecretArn, + // 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, + })); + secretDeleted = true; + } 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(linearWorkspaceId, oauthSecretArn, 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_arn: oauthSecretArn, + 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, + ); + } + logger.info('Linear OAuth secret already absent — treating removal as idempotent', { + request_id: requestId, + workspace_slug: slug, + }); + } + } + + // ─── 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: WORKSPACE_REGISTRY_TABLE, + 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_deleted: secretDeleted, + }); + + return successResponse(200, { + workspace_slug: slug, + linear_workspace_id: linearWorkspaceId, + status: purge ? 'purged' : 'revoked', + secret_deleted: secretDeleted, + }, 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. + */ +async function markSecretDeletionFailed( + linearWorkspaceId: string, + oauthSecretArn: string, + errorName: string | undefined, +): Promise { + await ddb.send(new UpdateCommand({ + TableName: WORKSPACE_REGISTRY_TABLE, + 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': oauthSecretArn, + }, + })); +} diff --git a/cdk/src/handlers/shared/response.ts b/cdk/src/handlers/shared/response.ts index 481211f7c..f59f4255e 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', SERVICE_UNAVAILABLE: 'SERVICE_UNAVAILABLE', INTERNAL_ERROR: 'INTERNAL_ERROR', diff --git a/cdk/test/constructs/linear-integration.test.ts b/cdk/test/constructs/linear-integration.test.ts index 450d1a25f..735d7df31 100644 --- a/cdk/test/constructs/linear-integration.test.ts +++ b/cdk/test/constructs/linear-integration.test.ts @@ -63,14 +63,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..0f4b78180 --- /dev/null +++ b/cdk/test/handlers/linear-remove-workspace.test.ts @@ -0,0 +1,403 @@ +/** + * 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({}); + }); +} + +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. + 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(JSON.stringify(updateCall![0].input)).toContain('revoked'); + expect(ddbSend.mock.calls.filter(([c]) => c._type === 'Delete')).toHaveLength(0); + + // Secret deleted. + const secretCall = smSend.mock.calls.find(([c]) => c._type === 'DeleteSecret'); + expect(secretCall).toBeTruthy(); + + const body = JSON.parse(result.body) as { data: { status: string; secret_deleted: boolean } }; + expect(body.data.status).toBe('revoked'); + expect(body.data.secret_deleted).toBe(true); + }); + + 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(JSON.stringify(updateCall![0].input)).toContain('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_deleted: boolean; status: string } }; + // Row still revoked; secret was already gone → reported as not-deleted-now. + expect(body.data.status).toBe('revoked'); + expect(body.data.secret_deleted).toBe(false); + }); + + 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. 404 keeps the endpoint from acting as a + // revoke-oracle 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' }); + }); + + 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 = ddbSend.mock.calls.find( + ([c]) => c._type === 'Update' + && c.input.TableName === 'LinearRegistry' + && JSON.stringify(c.input).includes('revoked'), + ); + expect(revokeUpdate).toBeTruthy(); + // ...and a durable secret-deletion-failed marker was persisted. + const marker = ddbSend.mock.calls.find( + ([c]) => c._type === 'Update' && JSON.stringify(c.input).includes('secret_deletion_failed'), + ); + expect(marker).toBeTruthy(); + }); + + 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. + const marker = ddbSend.mock.calls.find( + ([c]) => c._type === 'Update' && JSON.stringify(c.input).includes('secret_deletion_failed'), + ); + expect(marker).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'); + }); + + test('a registry row with no oauth_secret_arn skips the secret delete', async () => { + routeDdb({ registryRow: activeRow({ oauth_secret_arn: undefined }) }); + smSend.mockReset(); + + const result = await handler(makeEvent({ slug: 'acme', userId: ADMIN })); + expect(result.statusCode).toBe(200); + expect(smSend).not.toHaveBeenCalled(); + const body = JSON.parse(result.body) as { data: { secret_deleted: boolean } }; + expect(body.data.secret_deleted).toBe(false); + }); +}); diff --git a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts index 34fe749a2..b60a35d69 100644 --- a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts +++ b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts @@ -148,6 +148,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/cli/src/api-client.ts b/cli/src/api-client.ts index c48b1be53..786e4ed6e 100644 --- a/cli/src/api-client.ts +++ b/cli/src/api-client.ts @@ -40,6 +40,7 @@ import { GetPoliciesResponse, JiraLinkResponse, LinearLinkResponse, + LinearRemoveWorkspaceResponse, NudgeRequest, NudgeResponse, SlackLinkResponse, @@ -500,6 +501,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 aac4fe0a6..c71d194b7 100644 --- a/cli/src/commands/linear.ts +++ b/cli/src/commands/linear.ts @@ -1237,6 +1237,70 @@ 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 (preserving the audit trail) and the OAuth resolver + // fail-closes on any non-active status, so the workspace can no + // longer resolve a token or route webhooks the instant this returns. + // + // 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)}'`); + console.log(' • Leave project→repo mappings in place (remove those by project id)'); + 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'); + console.log(result.secret_deleted + ? ' ✓ OAuth secret deleted' + : ' • OAuth secret was already absent (nothing to delete)'); + console.log(' • Project→repo mappings left in place — remove by project id if needed'); + }), + ); + 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 30958bfda..fc034b29f 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -482,6 +482,20 @@ 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` for audit) or `purged` when the row was deleted outright + * (`--purge`). `secret_deleted` is false when the per-workspace OAuth secret + * was already absent (idempotent). Project→repo mappings are not touched (they + * carry no workspace id and are removed by project id). */ +export interface LinearRemoveWorkspaceResponse { + readonly workspace_slug: string; + readonly linear_workspace_id: string; + readonly status: 'revoked' | 'purged'; + readonly secret_deleted: boolean; +} + /** 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 54fb3043a..5232ebc5b 100644 --- a/cli/test/api-client.test.ts +++ b/cli/test/api-client.test.ts @@ -163,6 +163,42 @@ describe('ApiClient', () => { }); }); + describe('linearRemoveWorkspace', () => { + const okBody = { + ok: true, + json: async () => ({ + data: { + workspace_slug: 'acme', + linear_workspace_id: 'ws-1', + status: 'revoked', + secret_deleted: true, + }, + }), + }; + + 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..b8300a233 --- /dev/null +++ b/cli/test/commands/linear-remove-workspace.test.ts @@ -0,0 +1,164 @@ +/** + * 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: true, + }); + + 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: true, + }); + + 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: true, + }); + + 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_deleted: false)', async () => { + mockRemove.mockResolvedValue({ + workspace_slug: 'acme', + linear_workspace_id: 'ws-uuid-1', + status: 'revoked', + secret_deleted: false, + }); + + await runRemove(['acme', '--yes']); + const out = logSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toContain('already absent'); + }); + + // ─── 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: true, + }); + 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 423bd63ac..ff5797e61 100644 --- a/docs/guides/LINEAR_SETUP_GUIDE.md +++ b/docs/guides/LINEAR_SETUP_GUIDE.md @@ -203,9 +203,33 @@ Linear API rate limits per OAuth-installed app, per workspace: **5,000 requests/ Linear access tokens expire in 24h. The webhook processor and orchestrator auto-refresh via the stored `refresh_token` and write the rotated token back to Secrets Manager. If Linear returns `invalid_grant` (a concurrent caller already refreshed), the resolver re-reads the secret and uses the freshly-rotated token. -## 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` (preserves the audit trail). The OAuth resolver fail-closes on any non-`active` status, so the workspace stops resolving tokens and routing webhooks the instant the command returns. +- Deletes the per-workspace `bgagent-linear-oauth-` secret from Secrets Manager. + +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. + +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 \ @@ -216,7 +240,11 @@ aws dynamodb update-item \ --expression-attribute-values '{":removed":{"S":"removed"}}' ``` -Revoke a workspace install: +### Manual fallback + +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): ```bash aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery @@ -228,5 +256,3 @@ aws dynamodb update-item \ --expression-attribute-names '{"#s":"status"}' \ --expression-attribute-values '{":revoked":{"S":"revoked"}}' ``` - -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. diff --git a/docs/src/content/docs/using/Linear-setup-guide.md b/docs/src/content/docs/using/Linear-setup-guide.md index 31a3f5fa7..272adb8b5 100644 --- a/docs/src/content/docs/using/Linear-setup-guide.md +++ b/docs/src/content/docs/using/Linear-setup-guide.md @@ -207,9 +207,33 @@ Linear API rate limits per OAuth-installed app, per workspace: **5,000 requests/ Linear access tokens expire in 24h. The webhook processor and orchestrator auto-refresh via the stored `refresh_token` and write the rotated token back to Secrets Manager. If Linear returns `invalid_grant` (a concurrent caller already refreshed), the resolver re-reads the secret and uses the freshly-rotated token. -## 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` (preserves the audit trail). The OAuth resolver fail-closes on any non-`active` status, so the workspace stops resolving tokens and routing webhooks the instant the command returns. +- Deletes the per-workspace `bgagent-linear-oauth-` secret from Secrets Manager. + +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. + +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 \ @@ -220,7 +244,11 @@ aws dynamodb update-item \ --expression-attribute-values '{":removed":{"S":"removed"}}' ``` -Revoke a workspace install: +### Manual fallback + +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): ```bash aws secretsmanager delete-secret --secret-id bgagent-linear-oauth- --force-delete-without-recovery @@ -232,5 +260,3 @@ aws dynamodb update-item \ --expression-attribute-names '{"#s":"status"}' \ --expression-attribute-values '{":revoked":{"S":"revoked"}}' ``` - -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. 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