aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

fix(security): remove-workspace 403 is an existence oracle for active Linear workspaces — collapse non-admin responses (N9, follow-up to #306)

Open
#884 0 comments 0 reactions 0 assignees View on GitHub
adapters infra-cdk security
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

Parent context: surfaced during PR #681 review by @isadeks (issue #306), review [`5181793802`](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/681#pullrequestreview-5181793802) finding **N9**. Deferred from #681 deliberately because it changes API semantics; a bugfix PR is the wrong vehicle. **Needs maintainer `approved` before implementation (ADR-003).**

The *wording* half of N9 (narrowing the "not a revoke-oracle" comment so it no longer over-claims) landed in #681. This issue covers only the behavioural half.

## Finding

`DELETE /v1/linear/workspaces/{slug}` returns two distinguishable statuses to a caller who is **not** the workspace's installing admin:

- **403** when the slug names a workspace that exists and is `active` but was installed by someone else.
- **404** when the slug names nothing.

So a non-admin caller can enumerate slugs and learn *which Linear workspaces are onboarded and currently active* — an existence oracle. `workspace_slug` is the Linear `urlKey`, which is low-entropy and frequently guessable from a company name, so enumeration is cheap.

The endpoint is already correct about the narrower property it claims: it does not leak `revoked`-vs-missing, because the lookup filters `status='active'` and a revoked row therefore 404s exactly like a missing one. N9 is about `active`-vs-missing.

## Suggested fix (reviewer's wording)

> worth narrowing the wording, or collapsing non-admin responses to 404.

The wording change is done. The remaining option is to **collapse the non-admin 403 to 404**, so an unauthorised caller cannot distinguish "exists and active" from "does not exist".

## The tradeoff to settle in review

Collapsing to 404 is the standard anti-enumeration posture, but it degrades operator experience in the common non-attack case: a second admin on the same platform deployment who legitimately tries to remove a workspace onboarded by a colleague currently gets an actionable 403 and would instead get a bare "no such workspace". That is a genuine support cost, and it is why this was not smuggled into #681.

Options worth weighing:

1. **Collapse unconditionally to 404.** Simplest, strongest, worst operator experience.
2. **Keep 403 for platform admins, 404 for everyone else.** Preserves the actionable error for the population that can legitimately act on it, and removes the oracle for the population that cannot. Requires a platform-admin notion at this call site distinct from "installed this workspace".
3. **Keep 403 but log the denied probe** (`workspace_slug`, caller id) to CloudWatch so enumeration is at least detectable, and accept the oracle. Cheapest; does not close the finding.

Recommendation: option 2 if a platform-admin predicate is already available at the authorizer boundary, else option 1.

## Consistency check required

Whatever is chosen, it should be applied consistently with the sibling integration endpoints rather than only here — a 404-collapse on DELETE while another `/v1/linear/*` route still 403s on the same slug leaves the oracle intact by another door. Audit `cdk/src/handlers/linear-*.ts` for the same exists-and-active-vs-missing distinction before implementing.

## Acceptance criteria

- [ ] Decision recorded in the issue thread (which option, and why) before code.
- [ ] Chosen behaviour implemented at `cdk/src/handlers/linear-remove-workspace.ts`'s ownership check.
- [ ] Handler test asserting a non-owner caller cannot distinguish an existing `active` workspace from a nonexistent slug (identical status **and** identical body/error code).
- [ ] Sibling `/v1/linear/*` routes audited for the same oracle; findings either fixed here or filed separately.
- [ ] `docs/guides/LINEAR_SETUP_GUIDE.md` updated if the operator-visible failure mode changes, plus `cli/src/commands/linear.ts` messaging so "not found" does not read as a bug to a second admin.

Contributor guide

Open the contributing guide

Research direction

First read cdk/src/handlers/linear-remove-workspace.ts and record the maintainer-approved choice among the three response options. Audit cdk/src/handlers/linear-*.ts for the same oracle, then review the acceptance criteria for the handler test, docs/guides/LINEAR_SETUP_GUIDE.md, and cli/src/commands/linear.ts. Done means the chosen behavior and identical non-owner responses are tested consistently, with operator messaging updated if needed.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
api, authorization, backend, cloud, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.