airajena opened a new issue, #62:
URL: https://github.com/apache/fineract-business-intelligence/issues/62
## 🔍 Audit : Findings to Fix
### 🔴 Critical
| # | Finding |
|---|---|
| 1 | RLS fails open to `'admin'` when `current_username()` is empty |
| 2 | RLS injectable via unescaped username interpolation |
| 3 | Test suite conceals the RLS bug (strips the vulnerable line before
testing) |
| 4 | No database-level RLS (`CREATE POLICY`) — only app-layer SQL text |
| 5 | Historical portfolio figures wrong — current balance replicated across
all history |
| 6 | `loan_status_id = 300` filter makes closed loans vanish from history
retroactively |
| 7 | PAR bands based on bucket config (`min_age_days`), not real DPD |
| 8 | Per-batch commits break run atomicity (`WatermarkManager.update()`
committing mid-run) |
### 🟡 Smaller Items
| # | Finding |
|---|---|
| 9 | NULL cursor columns silently skipped, causing hard failures on NOT
NULL columns |
| 10 | COB gate too loose — any completed job satisfies a gate meant for
Close-of-Business specifically |
| 11 | Bind-parameter ceiling risk (~51k params against 65535 limit) |
| 12 | No TLS on database connections |
| 13 | Deletes never propagate — upsert-only, orphans persist forever |
---
## 📋 Fix Notes
### 🔴 Critical Fixes
**RLS (Findings 1–4)**
- Remove `or 'admin'` fallback from all six `superset/datasets/secure.sql`
files
- Escape username before string interpolation, or bind as a parameter
- Stop stripping the `{% set username = ... %}` line in
`test_rls_isolation.py` — the test must exercise the actual line
- Long-term: replace Jinja RLS with native Postgres `CREATE POLICY` +
per-user roles
> ⚠️ **Known gap:** Native Postgres RLS (`CREATE POLICY`) requires `SET
app.username` per query via Superset's `DB_CONNECTION_MUTATOR` hook. Risk of
session variable leaking across pooled connections could not be safely verified
— documented as a follow-up item.
**Historical Data (Findings 5–6)**
- Rebuild snapshots from transaction history (true point-in-time), not
current balances from `stg_m_loan`
- Drop the `loan_status_id = 300` filter for historical dates — closed loans
must stay in historical snapshots
**PAR Bands (Finding 7)**
- Derive DPD from `m_loan_repayment_schedule` (already extracted), not
bucket `min_age_days`
**Atomicity (Finding 8)**
- Remove `commit()` from `WatermarkManager.update()` — atomicity must be
owned solely by `run()`
### 🟡 Smaller Fixes
**Finding 9** — Coalesce NULL `last_modified_on_utc` to an epoch sentinel
before cursor comparison
**Finding 10** — Scope COB gate to `job_name` matching Close-of-Business
specifically, not any completed Spring Batch job
**Finding 11** — Chunk inserts to stay under Postgres's 65535 bind-parameter
limit
**Finding 12** — Add configurable `ssl_mode` per database connection
**Finding 13** — Implement reconcile mode to propagate hard deletes from
Fineract to the warehouse
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
