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)

Aperta
#884 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
adapters infra-cdk security
Lingua principale
TypeScript
Stelle
143
Fork
46
Merge medio
3g 10h
PR unite (30g)
24

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Per prima cosa leggi cdk/src/handlers/linear-remove-workspace.ts e annota la scelta approvata dal maintainer tra le tre opzioni di risposta. Verifica cdk/src/handlers/linear-*.ts per lo stesso oracle, poi esamina i criteri di accettazione per il test dell’handler, docs/guides/LINEAR_SETUP_GUIDE.md e cli/src/commands/linear.ts. Il lavoro è completato quando il comportamento scelto e le risposte identiche per i non proprietari sono testati in modo coerente e i messaggi per gli operatori sono aggiornati, se necessario.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
aws, typescript
Ambito
api, authorization, backend, cloud, security
Tipo di issue
Bug
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Attiva
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.