fix(security): push analytics tenant isolation into repository query - #342
fix(security): push analytics tenant isolation into repository query#342seonghobae wants to merge 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough분석 KPI 조회가 전체 작업 조회 후 필터링하는 방식에서 테넌트 범위 저장소 조회 방식으로 변경되었습니다. 저장소 계약과 인메모리 구현을 추가했으며, 권한 및 테넌트 경계 조건을 테스트합니다. Changes테넌트 범위 분석 조회
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyticsController
participant ConversionJobRepository
participant KPIRegister
Client->>AnalyticsController: KPI snapshot request
AnalyticsController->>ConversionJobRepository: findAllByTenantId(tenantId)
ConversionJobRepository-->>AnalyticsController: tenant-scoped ConversionJob list
AnalyticsController->>KPIRegister: build KPI snapshot
KPIRegister-->>Client: KPI snapshot response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Superseded by #432. Fresh reconstruction proved #342 had diverged one commit behind protected |
Objective
Reconcile the tenant-scoped list-query semantic from stale descendant #268 directly on current protected main and adopt it in analytics so KPI reads never request the global conversion-job inventory and filter ownership afterward.
Fresh exact state — 2026-08-10
4eeba73d70db172e44062891c01273efc4109d93;main:55d7ae8647208e301f282350f076eeddaba61d11;31401293063: success;31401293204: success;31401293014: success;31401293142: success;Test-first evidence
RED
Test-only head
0a5ff205b550e2435c4b9267dbc224a2f30e1f02introduced a focused controller boundary regression before production changes. Exact-head CI31400542789checked out that exact SHA and failed during Maven test compilation because protected main did not exposeConversionJobRepository.findAllByTenantId(String). The compiler reported the missing method at all three deliberate test references. Maven merge compatibility and fuzz failed at the same first boundary; buyer-readiness script tests remained green. This established that protected main still required a global inventory read for KPI calculation.GREEN
Current head adds a fail-closed repository contract whose default returns an empty list rather than falling back to global
findAll(). The process-local adapter overrides it with a tenant predicate and rejects missing/blank tenant identifiers.AnalyticsControllernow passes the authenticated tenant directly intofindAllByTenantId(...)and no longer materializes global job inventory.Focused tests prove:
findAll();Exact-head CI also proves synthetic-merge compatibility, packaging, the complete Java test suite, exact owned coverage and public Javadocs. Security Scan, Semgrep and fuzz are GREEN on the unchanged exact head.
Scope boundary
This is a deliberate current-base reconciliation of one valuable #268 semantic plus issue #326 analytics adoption. It does not copy #268's stale ancestry or claim the whole descendant is reconciled. Immutable identifier/tombstone behavior, scoped retry/mutation, durable deletion receipts/recovery, audit pseudonymization and generation fencing remain separate semantics to preserve or reimplement deliberately. It is path-disjoint from #341's administrative delete endpoint and from #338/#339 KPI formula/ledger semantics.
Merge gate
Move to Ready for ordinary independent review. Keep this exact head unchanged. Merge or auto-merge may complete only after live protection still sees all applicable required checks passing, zero valid unresolved findings, and any required qualifying independent non-author approval. Automated evidence is not approval.
Summary by CodeRabbit
개선 사항
테스트