Skip to content

feat(automation): finding lifecycle engine, scan outcome contracts, and pattern detection [1/5] - #326

Open
TFT444 wants to merge 8 commits into
devfrom
feat/311-finding-lifecycle
Open

feat(automation): finding lifecycle engine, scan outcome contracts, and pattern detection [1/5]#326
TFT444 wants to merge 8 commits into
devfrom
feat/311-finding-lifecycle

Conversation

@TFT444

@TFT444 TFT444 commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements the finding lifecycle foundation described in issue #311 (Automation PR 1/5).

  • 6 new DB tables via Alembic migration e1f2a3b4c5d6: scan_rule_outcomes, scan_lifecycle_applications, finding_fingerprints, finding_lifecycles, finding_lifecycle_transitions, patterns
  • LifecycleService.apply_scan(): idempotent, fail-closed, single transaction with FOR UPDATE row locking; a finding only resolves when its rule and collectors succeeded over the same authorised inventory boundary
  • PatternService.detect_and_publish(): detects three pattern types (persistent_finding, cross_resource_recurrence, reopened_finding) with deterministic thresholds stored in the record
  • Patterns API: GET /api/v1/patterns and GET /api/v1/patterns/<id> with tenant-scoped IDOR protection (subscription enforced in SQL, cross-subscription requests rejected 400/404)
  • Engine outcome recording: run_scan() now records SUCCESS / EMPTY_SUCCESS / PERMISSION_DENIED / TIMEOUT / FAILED per rule; Azure HTTP 403 detected as PERMISSION_DENIED
  • Worker wiring: LifecycleService and PatternService called immediately after db.save_scan(); lifecycle failures are non-fatal so the scan record is always preserved

Test plan

  • tests/test_finding_lifecycle.py — 18 unit cases covering idempotency, fail-closed behaviour, state transitions, and audit trail
  • tests/test_patterns.py — 12 cases covering all three pattern types, IDOR regression (no-subscription-400, cross-subscription-400, cross-subscription-IDOR-404)
  • Run pytest tests/test_finding_lifecycle.py tests/test_patterns.py -v locally (no live Postgres required; all DB calls mocked)
  • Confirm GET /api/v1/patterns returns 400 without a subscription scope
  • Confirm Alembic migration applies cleanly: alembic upgrade head

Notes

This is PR 1/5 for the finding lifecycle automation epic. Subsequent PRs will add remediation-agent integration, scheduled re-scan triggers, SLA tracking, and the prod-gate controls described in #311.

@TFT444 TFT444 changed the title feat(lifecycle): durable finding lifecycle and pattern detection (#311) feat(automation): finding lifecycle engine, scan outcome contracts, and pattern detection [1/5] Aug 30, 2026
@TFT444
TFT444 force-pushed the feat/311-finding-lifecycle branch from 629f8ab to c58e3c0 Compare August 30, 2026 14:54
TFT444 added 5 commits August 30, 2026 16:04
@TFT444
feat(lifecycle): implement finding lifecycle tracking (#311)
c0155f9 
Add six new tables (Alembic migration e1f2a3b4c5d6), two service
classes, a patterns API route, and engine outcome recording.

Tables added:
- scan_rule_outcomes: per-rule status per scan
- scan_lifecycle_applications: idempotency sentinel
- finding_fingerprints: stable immutable SHA-256 identity per finding
- finding_lifecycles: mutable OPEN/RESOLVED/ACCEPTED/SUPPRESSED/REOPENED state
- finding_lifecycle_transitions: append-only audit trail
- patterns: published persistent_finding / cross_resource_recurrence /
  reopened_finding detections

Services:
- LifecycleService.apply_scan(): idempotent, fail-closed, single transaction
  with FOR UPDATE row locking
- PatternService.detect_and_publish(): three pattern types with hardcoded
  thresholds stored in the record

Routes:
- GET /api/v1/patterns  (list with subscription_id / pattern_type / limit)
- GET /api/v1/patterns/<id>  (single pattern or 404)

Engine:
- ScanEngine.run_scan() now records per-rule outcome status
  (SUCCESS / EMPTY_SUCCESS / PERMISSION_DENIED / TIMEOUT / FAILED)

Tests:
- tests/test_finding_lifecycle.py: 18 cases (mocked DB, no live Postgres)
- tests/test_patterns.py: 9 cases (service unit + Flask route tests)

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix(security): close IDOR and tenant-bypass in patterns API
d3e6542 
GET /api/v1/patterns: the subscription scope is now always enforced in
SQL (WHERE subscription_id = %s, never a nullable IS NULL bypass). The
JWT subscription_id is the authority; a query-param that disagrees with
the JWT is rejected with 400. A request with no subscription_id in
either the JWT or the query param is also rejected.

GET /api/v1/patterns/<id>: the WHERE clause now includes subscription_id
so a caller cannot enumerate patterns from other subscriptions by ID.
An out-of-scope ID returns 404 to avoid disclosing that the pattern
exists in another subscription.

Adds three new regression tests: no-subscription-400, cross-subscription
query-param-400, and cross-subscription IDOR-404.

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix(lifecycle): address all code review findings from #311
fb50798 
Critical fixes:
- Wire LifecycleService.apply_scan() and PatternService.detect_and_publish()
  into scanner/worker.py immediately after db.save_scan(); lifecycle failures
  are non-fatal so the scan record is always preserved
- Write durable per-rule outcome rows to scan_rule_outcomes at the start of
  apply_scan so the audit record exists even if lifecycle processing fails
- Add subscription_id = effective_sub filter to GET /api/v1/patterns/<id>
  via _effective_subscription() helper (already present in patterns.py HEAD)

Important fixes:
- Add UNIQUE(pattern_type, lifecycle_id, scan_id) constraint to patterns
  table in migration; change ON CONFLICT DO NOTHING to ON CONFLICT ON
  CONSTRAINT uq_patterns_type_lifecycle_scan DO NOTHING
- Collect detection query results before closing RealDictCursor, then open
  fresh cursors for each _upsert_pattern call, eliminating nested-cursor
  overlap in pattern_service.py
- Narrow the absent-findings bulk lock to only rules with a resolving
  outcome (resolving_rule_ids); skip the query entirely when no rule
  produced SUCCESS/EMPTY_SUCCESS, reducing unnecessary lock contention
- Reset consecutive_success_count = 0 when a RESOLVED/ACCEPTED/SUPPRESSED
  finding is reopened
- Remove unused _BLOCKING_STATUSES frozenset from lifecycle_service.py
- Move 'import json' to module level in pattern_service.py

Test fixes:
- Refactor _FakeCursor/_FakeConn to use a shared deque so multiple
  cursor() calls on one connection consume from the same result stream
- Update scripted result sequences in all tests to include the new
  scan_rule_outcomes INSERT in the execute order
- Update fail-closed tests to reflect that no absent-findings query is
  issued when resolving_rule_ids is empty (FAILED/PERMISSION_DENIED)
- Add test_scan_rule_outcomes_written to verify audit record is emitted
- Add consecutive_success_count = 0 assertion to reopen test

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix(security): strip internal exception detail from all 500 error res…
4221dbd 
…ponses

Five routes were echoing str(exc) directly to the client in error
response bodies, exposing DB connection strings, filesystem paths, and
stack trace fragments. The JWT middleware was also leaking the specific
JWT validation failure reason via f-string interpolation.

Changes:
- api/app.py: InvalidTokenError now returns generic 'Invalid token' (no exc detail)
- api/routes/compliance.py: FileNotFoundError logs path, returns opaque message;
  general handler drops 'detail' field
- api/routes/findings.py: both endpoints drop 'detail: str(exc)' from 500 body
- api/routes/scans.py: all four catch blocks drop 'detail: str(exc)' from 500 body

Internal errors are still logged server-side at ERROR level.

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix(lifecycle): address all reviewer-blocking issues before merge
28968e9 
Critical:
- lifecycle_service: wrap entire transaction in try/except with explicit
  rollback() to prevent connection wedge on mid-transaction failure
- pattern_service: same rollback guard for detect_and_publish transaction
- lifecycle_service: replace NOT IN %s + tuple() with != ALL(%s) + list()
  to fix single-element tuple syntax error (PostgreSQL rejects trailing comma)
- patterns API: add tenant_id = %s to both SQL WHERE clauses to close
  cross-tenant pattern read in multi-tenant deployments

Important:
- patterns API: add _effective_tenant() helper using OPENSHIELD_TENANT_ID env
- migration: add 4 missing indexes (fingerprints tenant+sub, lifecycles state
  partial, transitions lifecycle_id, patterns sub+created_at)
- lifecycle_service: remove unused psycopg2.extras import
- test_finding_lifecycle: add test_rule_a_success_does_not_resolve_rule_b_finding
  to verify cross-rule resolution isolation (the key correctness invariant)
- test_finding_lifecycle: add rollback() to _FakeConn and _TrackingConn
- test_patterns: assert subscription_id appears in SQL params for IDOR test
  and subscription scoping test (not just mock return value)

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
TFT444 force-pushed the feat/311-finding-lifecycle branch from c58e3c0 to 28968e9 Compare August 30, 2026 15:04
@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

TFT444 added 3 commits August 30, 2026 16:21
@TFT444
fix(lint): remove unused imports and fix E402 in lifecycle test files
2bf7d94 
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix(lint): apply ruff format to all lifecycle PR files
4d78b62 
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444
fix: apply code review findings for lifecycle PR
26361c2 
- Add _VALID_OUTCOME_STATUSES allowlist; unknown status defaults to FAILED
  instead of letting DB constraint throw IntegrityError mid-transaction
- Fix dead-code if/elif block: restructure to if resolving_rule_ids / nested
  if seen_fingerprint_ids (fail-closed logic now clear at a glance)
- Add tenant_id to ix_patterns_sub_created index (was subscription_id only;
  multi-tenant queries filter on both columns)
- Add CASCADE to all downgrade() DROP TABLE statements (future FK safety)
- Add row_version comment clarifying it is a monotonic counter, not OCC
- Fix _FakeConn in test_patterns.py to share a single deque across all
  cursor() calls, matching test_finding_lifecycle.py and real psycopg2 behaviour
- Add REOPENED -> RESOLVED test covering finding absent from SUCCESS scan

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ritiksah141 ritiksah141 Awaiting requested review from ritiksah141 ritiksah141 is a code owner
@parthrohit22 parthrohit22 Awaiting requested review from parthrohit22 parthrohit22 is a code owner
@Vishnu2707 Vishnu2707 Awaiting requested review from Vishnu2707 Vishnu2707 is a code owner
@SHAURYAKSHARMA24 SHAURYAKSHARMA24 Awaiting requested review from SHAURYAKSHARMA24
@vogonPrayas vogonPrayas Awaiting requested review from vogonPrayas

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@TFT444