Skip to content

debt(arch): handlers query database.DB directly — ratchet it to zero and add a structural tenant-filter guard #808

Description

@alex-dembele

Problem

CLAUDE.md says handlers call use cases, and use cases call repositories that filter by tenant_id. On master that is only partly true, so tenant isolation depends on every author remembering the rule. Nothing in the structure enforces it.

Measured on 2026-09-25:

  • 100 direct database.DB calls in 14 handler files: team_handler.go (26), user_handler.go (19), mitigation_subaction_handler.go (12), mitigation_handler.go (9), stats_handler.go (8), saml2_handler.go (7), risk_handler.go (6), sso_session.go (5) and six files with one or two each.
    Command: grep -rn 'database.DB' backend/internal/handler --include='*.go' | grep -v _test
  • The contract location /internal/api/http/ holds 583 lines. internal/handler/ holds 22,895.
  • A second service layer, internal/service/, is 8.2k lines alongside internal/application/.
  • Some writes are filtered by id alone (internal UUIDs, not exploitable today, but a literal breach of rule 2):
    • gorm_risk_review_repository.go:43
    • gorm_evidence_repository.go:336
    • gorm_report_repository.go:75
    • mitigation_subaction_repository.go:190 (loads before checking)

#807 is what this structure produces: the handler itself was reasonable, but the effect crossed the tenant boundary and no structural guard caught it.

Acceptance criteria

  1. A CI step fails when the number of database.DB references under backend/internal/handler/ (tests excluded) goes up. The starting ceiling is 100, and the ceiling file is lowered in the same PR as any reduction, following the frontend/.lint-ceiling.json pattern (D-022).
  2. team_handler.go and user_handler.go (45 of the 100 calls) move behind use cases in internal/application/, with repositories that take tenantID. Each use case ships the three CLAUDE.md rule 4 tests.
  3. A GORM callback, enabled in tests and in staging, fails any SELECT, UPDATE or DELETE on a tenant-scoped table whose WHERE clause has no tenant_id or organization_id. The allowlist is explicit: each system query (workers, backfills) carries a one-line reason. go test ./... passes with the callback on.
  4. The four id-only writes above either carry the tenant or sit on the allowlist with their reason.
  5. Postgres Row-Level Security is not implemented here. It changes the tenant-isolation design, so it goes to the owner as D-055 in docs/DECISIONS.md.

Definition of Done

The ratchet is blocking in CI, the tenant guard is green over the full backend suite, and the before/after database.DB count is pasted on this issue.

Out of scope

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:backendGo, /internal, /pkgarea:qualityQuality, reliability and release readinesspriority:P2Normalpriority:P2-mediumNormal milestone workstatus:readyMeets the ready definitiontier:0-trustTrust: security, isolation, evidence integritytype:debtTechnical debt

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions