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)

Offen
#884 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
adapters infra-cdk security
Vorherrschende Sprache
TypeScript
Sterne
143
Forks
46
Ø Merge
3 T. 10 Std.
Gemergte PRs (30 T.)
24

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Lies zuerst cdk/src/handlers/linear-remove-workspace.ts und halte die vom Maintainer bestätigte Auswahl unter den drei Antwortoptionen fest. Prüfe cdk/src/handlers/linear-*.ts auf dieselbe Referenz und sieh anschließend die Akzeptanzkriterien für den Handler-Test, docs/guides/LINEAR_SETUP_GUIDE.md und cli/src/commands/linear.ts durch. Erledigt ist die Aufgabe, wenn das ausgewählte Verhalten und identische Antworten für Nicht-Eigentümer konsistent getestet werden und die Meldungen für Operatoren bei Bedarf aktualisiert sind.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
aws, typescript
Bereich
api, authorization, backend, cloud, security
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Aktiv
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.