Skip to content

feat: isolate logs and jobs by resolved scope (#17) - #45

Merged
cursor[bot] merged 3 commits into
mainfrom
cursor/scope-isolation-1cc6
Aug 17, 2026
Merged

cursor[bot] merged 3 commits into
mainfrom
cursor/scope-isolation-1cc6

Conversation

@leo-aa88

Copy link
Copy Markdown
Member

Closes #17

Summary

A shared service must not let one incident's logs contaminate another's baseline or analysis. This adds G8 scope isolation: every service read and write is stamped and filtered by a resolved scope, including baseline comparison.

Keys are pinned by default; --allow-scope-override lets a caller pass an explicit request scope. CLI stays optional and defaults to default. A service request with no resolvable scope returns 400 SCOPE_REQUIRED.

Data model

Migration 0008_scope_isolation:

  • ingestion_jobs.scope (existing rows → default)
  • Composite indexes on log_entries: (scope, timestamp) and (scope, service, environment, fingerprint) (existing indexes kept)
  • api_keys.allow_scope_override (default false / pinned)

Behavior

  • Auth off: request scope or default (existing TestClient tests keep working)
  • Pinned key/OIDC: always the key/principal scope; different request scope → SCOPE_MISMATCH
  • Override-allowed keys: request scope or key scope; empty → SCOPE_REQUIRED
  • Query filter helper threaded through clustering, baseline, evidence, ask, ingest persist/list, and CLI --scope

Tests

tests/unit/test_scope.py covers resolution, SQL compile isolation, and TestClient pin/override/list isolation. python -m pytest tests/unit/ passed locally (641).

Deferred

  • OIDC stays pinned to scope="default" (no invented claims)
  • Live-Postgres two-tenant integration tests (unit tests cover SQL + mocked TestClient)
  • cluster_runs / explanations tables are not scope-columned; explain cache key now includes scope
Open in Web Open in Cursor 

cursoragent and others added 2 commits August 17, 2026 09:02
Stamp ingestion jobs and filter every service read/write, including
baseline comparison, so one incident cannot contaminate another.

Closes #17

Co-authored-by: Leonardo <leo-aa88@users.noreply.github.com>
Keep lint clean on files touched for G8 so CI ruff check passes.

Co-authored-by: Leonardo <leo-aa88@users.noreply.github.com>
@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review — PR #45 (G8 scope isolation)

G8 isolation is implemented correctly on the paths that matter: migration 0008_scope_isolation (0007 untouched), job scope + the two log_entries composite indexes, key pin/override resolution, query helpers on clustering / baseline / evidence / ask / ingest list+latest, CLI default default, and AUTH_ENABLED=false still resolving to default. python -m pytest tests/unit/ → 641 passed. Ruff on touched files is clean.

Must-fix

(None)

Should-fix

  1. src/api/routes/ingestions.py:312 (_replay_if_active) — Idempotency lookup is still global (ingest_idempotency_keys.key PK, no scope). A caller in incident:B who reuses incident:A’s Idempotency-Key gets a 202 with A’s worker_job_id / ingestion_job_id instead of a new job or a scoped 409. Follow-up GET /v1/ingestions/jobs/{id} then 404s (:621) because payload scope ≠ B, so B cannot poll and never ingests. Reject replay when the stored job’s scope ≠ resolved scope, or unique the key per scope.

Nice-to-have

  1. src/api/routes/ingestions.py:276 — _job_matches_scope treats a missing/non-string job.scope as a match (for MagicMocks). After 0008 the column is NOT NULL, so this never fires in production; prefer SQL IngestionJob.scope == scope so mocks cannot accidentally skip isolation.
  2. src/core/ingestion/tail.py:241 — Tail ticks stamp lines from metadata_json["scope"] rather than job.scope. They are set together today; using the column would survive a metadata rewrite.
  3. src/worker/runner.py:94 — resume_ingestion_job_id is loaded by id only. A caller who already knows another scope’s UUID can copy its cursors. Check prior.scope against the payload scope.
  4. src/api/routes/explain.py:5 — Module docstring still says the cache key is (window, service, env, ingestion_job_id); _cache_key now includes scope.

Verdict

Needs changes (must-fix and/or should-fix remain)

A shared key is still global (PK on key only). Replaying it when the
stored job's scope differs from the caller now returns 409 instead of
another tenant's job ids. Tail/GET lookups use the scope column; resume
jobs are rejected across scopes.

Co-authored-by: Leonardo <leo-aa88@users.noreply.github.com>
@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review — PR #45 (G8 scope isolation, round 2)

Round 1 should-fix is actually fixed. _replay_if_active and the unique-key race in _bind_idempotency_key compare the stored job’s scope to the caller (src/api/routes/ingestions.py:349-404). Mismatch returns 409 IDEMPOTENCY_SCOPE_CONFLICT with no worker_job_id / ingestion_job_id. Same-scope still 202. Covered by test_cross_scope_key_reuse_does_not_return_other_scopes_job and test_same_explicit_scope_still_replays. Global key uniqueness remains (by design); it no longer leaks another scope’s job ids.

Issue #17 acceptance holds: 0008_scope_isolation (0007 untouched), job scope + log-entry composite indexes, pin/override, SCOPE_REQUIRED, CLI default default, baseline/query filters, cross-scope unit tests.

python3 -m pytest tests/unit/ → 644 passed. Ruff on the PR’s touched files is clean.

Must-fix

(None)

Should-fix

(None)

Nice-to-have

  1. src/api/routes/ingestions.py:497-501 and :516-517 — OpenAPI Idempotency-Key description and the route docstring still say a repeat always returns the original 202. README already documents 409 IDEMPOTENCY_SCOPE_CONFLICT. Docs-only; does not leak ids.

Verdict

Ready to merge (0 must-fix, 0 should-fix)

@cursor
cursor Bot merged commit a0181ae into main Aug 17, 2026
2 checks passed
@leo-aa88
leo-aa88 deleted the cursor/scope-isolation-1cc6 branch August 17, 2026 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(G8): scope isolation / multi-tenancy

2 participants