Comprehensive hardening + 50/30/20 Budget Rule page#202
Merged
Conversation
Two live 404s surfaced by audit: - Settings > AI Assistant "reveal API key" click hit /api/preferences/ai-config/key while backend serves it at /api/preferences/ai-config/key/reveal. - EPF/PPF/NPS instrument rate widgets hit /api/rates/instruments while rates.py mounted at /rates (no /api prefix). Aligned frontend to backend for the reveal path (backend keeps the explicit /reveal verb -- signals sensitivity). Aligned backend to frontend for rates.py, plus normalized exchange_rates.py and stock_price.py to /api/* prefix so all three pick up the no-store Cache-Control middleware in main.py (previously bypassed). Updated the corresponding call sites in frontend/services/api/ preferences.ts (getStockPrice, getExchangeRates) and refreshed docs/API.md.
Six parallel research agents surveyed: - Latest versions of every stack piece (25+ packages) - Feature ideas across 10 competitors (Monarch, YNAB, Copilot, Rocket Money, Empower, Firefly III, Actual Budget, Maybe, Cushion, Simplifi) - Robust anomaly detection (median+MAD, IQR, seasonality) - Postgres-in-CI, HTTP TestClient scaffolding, coverage thresholds - Security rollout: refresh rotation, encryption key split, KDF correctness (HKDF not Argon2id for KEK material) - New AI tools (tax simulation, subscription dedup, FIRE projection) Merged the findings into an ordered action plan across Waves 2-6. Wave 2 (structural fixes) is next.
Python version was declared in four different places and drifted: - pyproject requires-python >=3.11 - mypy python_version 3.12 - ruff target-version py313 - ci.yml python 3.13 - migrate.yml python 3.14 Standardize on 3.13 across pyproject + mypy + migrate.yml so tests and prod-migration run on the same interpreter (ruff already targets py313; ci.yml already runs 3.13). 3.14 stays on the shelf until the ecosystem catches up. Also: - migrate.yml: add backend/src/ledger_sync/db/_models/** to paths -- real model files live there now, not just the facade db/models.py, so schema changes weren't triggering the migration workflow. - All four workflow jobs get a timeout-minutes: no job had one, so a hung uv sync or alembic upgrade could burn 6 hours of runner time. - Fix keepalive.yml comment that referenced /api/health -- the actual endpoint is /health (app-level route in main.py, no /api/ prefix).
…or v2
Previously the JWT secret triple-duty'd as JWT signer, OAuth state
HMAC key, and BYOK KEK for at-rest API key encryption. Rotating the
JWT secret for token-security reasons would silently brick every
stored BYOK key (already caught + surfaced as DecryptionError, but a
user-visible foot-gun).
Fixes:
- New env var LEDGER_SYNC_ENCRYPTION_KEY, min 32 chars if set. Falls
back to jwt_secret_key during rollout with a deprecation warning.
- New v2 ciphertext format (0x02 prefix) using HKDF-SHA256, which is
the correct primitive when the input material is already a strong
key. PBKDF2's iteration knob only helps against brute-force of
low-entropy inputs -- a 32-byte server secret doesn't need it, and
the OWASP 600k bump / Argon2id switch is cargo-culting the password
case. HKDF is ~free CPU and RFC 5869 explicitly names KEK derivation
as its target use case.
- Legacy v1 blobs (unprefixed, PBKDF2) still decrypt. `decrypt_api_key`
now returns `(plaintext, needs_reencrypt)`; the caller in
`preferences_ai.get_ai_key` re-encrypts as v2 in-band on next read,
so stored ciphertexts transparently upgrade the next time a user
reveals their API key.
- After ~30 days of clean v1 access logs, the legacy branch and the
jwt_secret_key fallback can be removed.
uv.lock updated to reflect requires-python bump from 3.11 to 3.13.
Tests: 7 encryption tests, including one that recreates the exact v1
byte layout and proves the fallback branch decrypts + flags upgrade.
243 backend tests pass total (0 regressions).
Rollout:
1. Deploy code (this commit). Existing users unaffected -- v2 writes
use jwt_secret_key material until step 2.
2. Generate + set LEDGER_SYNC_ENCRYPTION_KEY on Vercel:
python -c 'import secrets; print(secrets.token_urlsafe(32))'
3. Wait 30 days -- v1 blobs re-wrap on next read.
4. Drop the jwt_secret_key fallback + _decrypt_v1 branch.
Previously /api/auth/logout was cosmetic -- the docstring literally admitted the access token stayed valid until natural expiry and no refresh-token revocation existed. A leaked token survived logout, account reset, and even account delete (up to 30min access + 7d refresh window). Fix: bake a `tv` (token_version) claim into every JWT, mirroring User.token_version. verify_token checks tv == user.token_version; bumping the column invalidates every outstanding token for that user in one integer write, no per-token blocklist needed. - Alembic migration: users.token_version INT NOT NULL DEFAULT 0. - create_tokens takes token_version, bakes it into JWT payload. - verify_token accepts expected_tv; missing tv treated as 0 during rollout, rejected under LEDGER_SYNC_JWT_STRICT_TV. - get_current_user does a second-pass verify with expected_tv. - refresh_tokens re-verifies with expected_tv so bumped tv kills refresh tokens too, not just access. - AuthService.logout bumps tv. reset_account bumps tv. - Hardened verify_token: int(sub) catches TypeError/ValueError instead of propagating as 500. Rollout: - Day 0: deploy code + migration. New tokens tv=0. Legacy tokens (no tv claim) soft-accepted as tv=0. - Day 8: set LEDGER_SYNC_JWT_STRICT_TV=true on Vercel. Any surviving pre-migration token now fails; user re-logs in once. Tests: 5 new tests, 248 backend tests pass total.
The existing `limiter` is IP-keyed via get_remote_address. Behind CGNAT, corporate NAT, or mobile carriers, every user on that egress shares one bucket -- a single misbehaving neighbor drains the quota for everyone. And behind Vercel's edge, IP-based limiting can end up keyed on the platform IP itself. Adds `user_limiter` keyed on the JWT `sub` claim, falling back to IP when the token is missing or malformed. Both limiters share the same in-memory backend and the same slowapi 429 handler; decorator stacking means whichever trips first returns 429. Applied on the two endpoints where per-user granularity matters: - /api/upload: user_limiter 10/min + limiter 50/min. User cap is the primary throttle; IP cap catches unauthenticated floods. - /api/ai/bedrock/chat: user_limiter 30/min + limiter 60/min. Same pattern -- protect the shared Bedrock quota per-user. Pre-auth endpoints (OAuth callback, /api/auth/refresh) keep IP-only limits since they run before we have a verified identity. Tests: 5 new tests covering token parsing, IP fallback, malformed tokens, non-Bearer schemes, and case-insensitive Bearer prefix. 253 backend tests pass total.
The audit found that 19 of 21 user_id foreign keys had no DB-level
ondelete rule -- cascade worked ONLY through the ORM relationship
config. Any raw-SQL user delete (via psql, a maintenance script, or
an admin tool bypassing the ORM) would orphan children in most tables.
Only AuditLog and InvestmentHolding had ondelete='CASCADE' declared.
Fixes:
- All 19 previously-naked user_id FKs across 6 model files now declare
ondelete='CASCADE': ai_usage, analytics (7), investments (2 more),
planning (5), transactions (3), and UserPreferences.
- Anomaly.transaction_id -> transactions.transaction_id also gets
ondelete='CASCADE' so hard-deleting a transaction doesn't raise
IntegrityError from stale anomaly rows.
- Alembic migration rewrites every FK with dialect-aware code paths:
* Postgres: discover existing constraint name via information_schema,
drop by name, recreate with ondelete.
* SQLite: batch_alter_table recreates the table with the new FK
(SQLite doesn't support ALTER TABLE ... DROP CONSTRAINT).
- ColumnMappingLog is the sole model without a user_id column (audit
finding). Left for a follow-up -- it's import-time telemetry, no
live production data goes through it.
253 backend tests pass, zero regressions.
… baseline
Rewrote the two anomaly detectors around robust statistics.
## _detect_high_expense_months: mean+stdev -> median+MAD
The historical mean+stdev approach self-masked. Example: 12 months at
50k INR + one at 150k gave mean=57.7k, stdev=27.7k. With the default
2.0-sigma cutoff, threshold was 113k -- the 150k outlier tripped it by
inches, and a slightly stricter user preference (2.5) would miss it
entirely because the outlier itself inflated both mean and stdev.
New path: Iglewicz-Hoaglin modified Z-score
|0.6745 * (x - median) / MAD| with a NIST-recommended 3.5 cutoff.
MAD has a 50% breakdown point vs stdev's 0%. When MAD collapses to 0
(>=50% identical values), falls back to Tukey's upper IQR fence
Q3 + 1.5*IQR (25% breakdown, still useful).
## _detect_large_transactions: all-time avg -> rolling median
Was comparing each txn against the all-time category avg. A big
legitimate 2023 purchase inflated the baseline forever -- normal-size
2026 txns looked small, genuinely large ones got graded less severely.
New path: per-category 12-month rolling window, median baseline,
leave-one-out (window strictly excludes the txn under test). MIN_
HISTORY=5 warmup stops false-positive spam on new users' first upload.
## Threshold preference migration (backward compat)
Existing users have anomaly_expense_threshold=2.0. Interpreting that
as-is under modified-Z would be much stricter. Instead map anchored:
effective_z_cutoff = 3.5 * (stored / 2.0)
Default 2.0 -> 3.5 cutoff (baseline), 2.5 -> 4.375 (stricter), 1.5 ->
2.625 (looser). Same knob, better math, zero migration.
Tests: 6 new unit tests on pure math (self-masking regression, IQR
fallback, formula sanity, MAD stability). 3 integration tests renamed
kwarg threshold_multiplier -> z_cutoff. 259 backend tests pass.
…vate config
Three small structural fixes bundled:
1. FY-summary tax detection: the regex was `\btax(es)?\b` which caught
English "tax" but missed the actual Indian tax vocabulary users
write in bank statements. Users with GST payments, TDS deductions,
cess reversals, surcharge line items, self-assessment tax, or
advance-tax quarterly outflows had their tax_paid on the FY summary
silently understated. Broadened to
\b(tax(es)?|tds|gst|cess|surcharge|advance[\s-]?tax|self[\s-]?assessment)\b
Word boundaries still exclude Taxi / Syntax / Cesspool. 14 new
parametrized tests lock the behavior in.
2. backend/Makefile deleted. It referenced Poetry (`poetry install`,
`poetry run pytest`) and black -- the project uses uv + ruff. Running
`make install` on a fresh clone would have failed. Also had a
checkmark emoji in the success output that contradicted the workspace
no-emoji rule. uv commands live in CLAUDE.md; the Makefile added
nothing.
3. .github/renovate.json deleted. Root renovate.json extends the shared
`github>Sagargupta16/shared-workflows` preset -- that's the live
config. The .github/ one duplicated it with a slightly different
schedule and was dead code (Renovate reads root-level first). Two
competing configs was pure drift risk.
273 backend tests pass (+14 tax regex).
GET /api/analytics/v2/spending-rule?start_date=&end_date=
Returns per-category monthly averages classified into Needs / Wants /
Savings buckets over a user-selected date range, plus header totals and
delta-vs-target scores. Powers the new /budgets page (replaces the
per-category cap manager).
## Response shape
{
"period": {"start", "end", "months"},
"income_total", "expense_total", "savings_amount",
"targets": {"needs": 50.0, "wants": 30.0, "savings": 20.0},
"buckets": {
"needs": {"amount", "pct_of_income", "score_delta"},
"wants": {"amount", "pct_of_income", "score_delta"},
"savings": {"amount", "pct_of_income", "score_delta"},
},
"categories": [{category, subcategory, bucket, total_amount,
avg_monthly, txn_count, months_seen}, ...],
}
- savings_amount uses the Warren definition (income - expenses)
- Category rows in the "savings" bucket show what was explicitly moved
to investment accounts (SIP, PPF, EPF, etc.)
- avg_monthly divides by months-in-period so an annual bill shows its
true monthly-equivalent
- score_delta is signed so positive = on the good side of target
## Bucket classification
Reads user's essential_categories + investment_account_mappings prefs;
falls back to opinionated Indian defaults when either is empty:
- Needs defaults: Rent, Housing, EMI, Utilities, Groceries, Food &
Dining, Fuel, Transport, Insurance, Healthcare, Medicine, Education,
Family Support, Internet, Phone.
- Savings-account patterns: SIP, MF, PPF, EPF, NPS, Stocks, ELSS, RD,
SSY, plus brokers (Groww, Zerodha, Kite, Upstox, Kuvera, Coin).
- Wants = residual.
## Tests
8 FastAPI TestClient integration tests -- first HTTP-boundary tests in
this repo, per the audit's coverage-gap finding. Used StaticPool +
check_same_thread=False so in-memory SQLite is shared between the
fixture thread and the TestClient handler thread. 281 backend tests
pass total.
Rip-and-replace of the per-category cap manager. New page shows the
Warren 50/30/20 breakdown of income across Needs / Wants / Savings,
plus a category-level monthly-average table grouped by bucket.
## Page structure
Header row: three bucket cards (Needs / Wants / Savings)
- Big amount + % of income
- Progress bar with target tick
- Signed delta vs target ("+16 pts" for over-savings, "-5 pts" for
over-needs). Green = on target; amber = within 5 pts; red = 5+ off.
Table below: category rows grouped by bucket, sortable within each
group, shows category / avg-per-month / total-over-period / % of income
/ txn count. Section header shows the bucket-wide monthly average.
Period picker: Last 3 mo / Last 6 mo / Last 12 mo (default) / This FY
(Indian April-March fiscal year).
## Files
- frontend/src/pages/budget/BudgetPage.tsx: rewrite -- thin
orchestrator that fetches spending-rule data and hands it to
BucketCard + CategoryTable.
- frontend/src/pages/budget/budgetUtils.ts: rewrite -- period -> ISO
date-range mapper. Old STATUS_CONFIG deleted (was for per-category
cap grading which no longer exists here).
- frontend/src/pages/budget/components/BucketCard.tsx: new -- the big
three cards. Uses ProgressBar with target tick.
- frontend/src/pages/budget/components/CategoryTable.tsx: new --
grouped DataTable, sortable within each bucket.
- frontend/src/pages/budget/components/PeriodPicker.tsx: new -- pill
selector matching the existing /budget viewMode tab style.
- frontend/src/pages/budget/{useBudget.ts, types.ts,
components/AddBudgetForm.tsx, BudgetCharts.tsx,
BudgetDefaultsPanel.tsx, BudgetRowItem.tsx}: deleted -- old
per-category cap flow.
- frontend/src/services/api/analyticsV2.ts: added SpendingRule*
types + service method.
- frontend/src/hooks/api/useAnalyticsV2.ts: added useSpendingRule hook.
- frontend/src/components/layout/Sidebar/navConfig.ts: label
"Budget Manager" -> "Budget Rule".
## Notes
- Budget model + /api/analytics/v2/budgets endpoints stay live -- the
MobileTabBar and Sidebar still use them for over-budget badges.
Only the /budgets page UI changed.
- Type-check clean, ESLint clean, 266 frontend tests pass, 281 backend
tests pass.
… bad response shapes
Local end-to-end run against SQLite dev DB exposed two real bugs.
## 1. cascade_user_fks_2026 migration left duplicate FKs on SQLite
Applied against a live pre-migration DB: `PRAGMA foreign_key_list`
after upgrade showed EVERY affected table had two FKs to users.id
(one old NO ACTION, one new CASCADE). Cause: the old batch used
`create_foreign_key` without a matching `drop_constraint`; on SQLite
the reflected unnamed / anonymous FK survived alongside the new one,
because batch_alter_table treats create-without-drop as APPEND, not
REPLACE. Anomalies got 3 FKs total (2 to transactions + 1 to users).
Fixed by reflecting each table under an alembic naming_convention
first (so anonymous FKs get synthesizable names in reflection),
building a (local_col, referred_table) -> current_name map from the
reflected metadata, then dropping by that name inside the batch
before the new CASCADE FK is created. After the fix all 19 tables
have exactly one CASCADE FK to users; anomalies has exactly 2 (users +
transactions, both CASCADE).
Verified by running upgrade on a full copy of the local dev DB and
inspecting PRAGMA foreign_key_list on every affected table.
## 2. /budgets page crashed with "Cannot read properties of undefined (reading 'start')" in demo mode
The demo-mode axios adapter in client.ts had a catch-all for
`/analytics/v2/*` that returned `{data: [], count: 0}` -- correct
shape for the other v2 endpoints, wrong for the new /spending-rule
which returns `{period, buckets, categories, ...}`. Frontend then
read `data.period.start` on the malformed object and crashed the
whole render tree.
Two-part fix:
- Add a proper demo stub for /spending-rule returning a zero-state
response with the real shape. Demo users see an empty history state
instead of a crash.
- Defensive shape check in BudgetPage: if `data` is truthy but missing
`period` or `buckets`, render the EmptyState error card rather than
attempting to render BudgetRuleContent. Fails soft on any future
shape drift (bad server response, cached wrong data, etc).
Type-check + ESLint clean, 266 FE tests pass, 281 BE tests pass.
`const capForBar = kind === 'cap' ? target : target` returned the same value from both branches -- Sonar flagged as a MAJOR reliability bug (C rating on new code, tripped the quality gate). The variable was only used in one Math.max() call; inlined `target` and dropped the variable. Behavior unchanged. FE 266 tests pass, type-check + ESLint clean.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ions Visual + UX improvements to the 50/30/20 Budget Rule page. ## Three columns side-by-side Previously the Needs / Wants / Savings breakdown was three sections stacked vertically -- hard to compare, scrolling-heavy. Now they sit as three glass-card columns in a `grid-cols-1 lg:grid-cols-3` grid: mobile keeps the stack (unreadable side-by-side on phone per the repo's mobile-first rule), tablet stacks 1-1-1, desktop shows all three at once. ## Top 10 + Other (N more) rollup Each column caps at 10 category rows. Rows 11+ get folded into a single "Other (N more)" row summing avg_monthly / total / txn_count. Clicking the row expands an inline list of what was rolled up so users don't lose visibility of the tail. Rationale: a user with 30 dining subcategories was making the Wants column an ugly scrolling wall next to a tidy 4-row Savings. Top-N with rollup keeps the three tables visually balanced without hiding data. ## Period picker expanded Added preset chips: 2 yr, 5 yr, All (uses data-date-range endpoint's min/max), plus a new "Custom" chip with a date-range popover. From/To inputs are HTML `<input type="date">` (matches existing app convention in TransactionFilters/settings/goals). Apply button commits the range; Escape or outside-click closes the popover. Custom start/end are capped by minDate/maxDate from useDataDateRange when available. Backend already accepts arbitrary start_date/end_date on the /spending-rule endpoint -- no API change needed. ## Files - components/PeriodPicker.tsx: expanded PresetPeriod union + custom popover with dual date inputs. - components/CategoryTable.tsx: three-column grid layout, TOP_N rollup with expandable "Other" row, per-column DataTable. - BudgetPage.tsx: wires custom range state + useDataDateRange bounds. - budgetUtils.ts: extended toPeriodRange with new cases (2yr/5yr/ all_time/custom); custom passes through the YYYY-MM-DD inputs. FE 266 tests pass, type-check + ESLint clean.
When a user's Excel labels investment moves as generic "Transfer" (both
category AND subcategory), the /budgets Savings column read literally
as "Transfer" -- confusing, because that money didn't go to another
wallet, it went into PPF/SIP/EPF/etc.
Fix: display-side rename in the /spending-rule endpoint. Savings-bucket
rows whose category is a generic Transfer-family label ("transfer",
"transfer out", "transfer to", "movement", "internal transfer", or
empty) get relabeled based on the destination account:
to_account contains "ppf" -> "PPF Contribution"
to_account contains "epf" -> "EPF Contribution"
to_account contains "nps" -> "NPS Contribution"
to_account contains "ssy" -> "Sukanya Samriddhi"
to_account contains "elss" -> "ELSS Investment"
to_account contains "mf" -> "Mutual Fund Investment"
to_account contains "sip" -> "SIP Investment"
to_account contains "groww/zerodha/kite/upstox/coin/kuvera" -> instrument
to_account contains "rd" -> "Recurring Deposit"
to_account contains "fd" -> "Fixed Deposit"
(unknown destination) -> "Investment" (fallback)
Only affects display. DB rows untouched, other analytics unchanged.
Non-generic categories (e.g. "Investment", "PPF") stay as-is; the
prettifier only fires on the generic "Transfer" labels.
Tests: 2 new integration tests -- one asserts generic Transfer +
known destinations map to instrument names, one asserts unknown
destinations fall back to "Investment" instead of leaving "Transfer".
283 backend tests pass total (+2).
Before: /budgets grouped by (category, subcategory) so a user with 7
Food & Dining subs saw 7 separate rows dominating the Needs column,
crowding out other categories. Reference budget mocks show one row per
category with sub-breakdown inline -- much cleaner.
Backing data (survey of local dev DB, 6768 txns): Food & Dining alone
has 7 subs / 2510 txns. Housing 7 subs, Miscellaneous 6 (of which 587
of 659 are 'Uncategorised' -- a data-quality signal the user should
see). Six of 12 expense categories have 5+ subs. Collapsing pays off.
## Backend changes
- Aggregate by (category, bucket) instead of (category, subcategory,
bucket). One row per category per bucket.
- _CategoryRow.add() rolls up amounts by subcategory into a dict; on
serialization, emit top-3 subs by amount desc as
`top_subs: [{name, amount}, ...]`.
- `subcategory` field kept in response for backward compat but always
None under the new grouping. Callers should read `top_subs`.
## Prettifier bug fix
`_prettify_savings_label` used to relabel unknown-destination Transfer
rows as generic "Investment" -- wrong for edge cases like
`Cashback Shared` (117 rows) and `Security Deposits` (5 rows). Now
returns the ORIGINAL category on unknown destinations. Wallet-to-
wallet skip filters most of these earlier; this is a safety net.
## Frontend changes
CategoryTable renders a small second line under each category name
listing top-3 sub names separated by `·`. `title` tooltip shows the
full name+amount breakdown on hover. `(no subcategory)` labels are
filtered out of the display line.
Tests: 2 new integration tests (sub-collapse, top-3 cap). 285 backend
tests pass total (+2). Frontend type-check + lint clean, 266 tests
pass.
Two issues from the screenshot: ## 1. Savings column showed raw 'Transfer: Bank: X → Y: Z' rows The prettifier only fired when the category was a single generic word like 'Transfer'. The Excel template stores TRANSFER rows as compound 'Transfer: Bank: HDFC → Stocks: Groww' -- the prettifier saw this as non-generic and left it alone. Fix: broaden the generic-transfer detection to also catch any category starting with 'transfer:' (colon-prefix). That's the ledger-sync default template's format, and it maps 1:1 to the to_account column which is what the prettifier keys on. So 'Transfer: Bank: HDFC → Stocks: Groww' with to_account='Stocks: Groww' now correctly relabels to 'Stocks'. Also shortened the labels per user request: PPF Contribution -> PPF EPF Contribution -> EPF Mutual Fund Investment -> Mutual Funds Stocks Investment -> Stocks SIP Investment -> SIP ELSS Investment -> ELSS Plus added FD/Bonds explicit label. ## 2. Substring FP: 'rd' matched inside 'weird broker' `_classify_category` used `pattern in to_lower` -- 'rd' (Recurring Deposit) matched 'weird broker xyz' and pushed the row into Savings. Same for 'fd' vs 'fedex', 'mf' vs random substrings. Fix: word-boundary matching via `\b<pattern>\b` regex. Multi-word patterns like 'mutual funds' fit naturally on space boundaries. Applied to both `_classify_category` and `_prettify_savings_label`. ## 3. Horizontal scroll on the three side-by-side columns DataTable defaults to overflow-x-auto -- correct for TransactionsPage, wrong for narrow budget columns. Wrapped each column's DataTable in a container that overrides the scroll class with overflow-x-hidden. Text truncation via existing truncate + min-w-0 on the category cell handles overflow gracefully. Now that transfer rows are short instrument names, truncation rarely fires in practice. Tests: 1 new integration test locks the compound-Transfer relabel path against regression. 2 existing tests updated for the shortened labels. 286 backend tests pass (+1). 266 frontend tests pass, type-check + lint clean.
Deep audit found the budget page was flat/broken because I used a CSS class that doesn't exist in the design system. ## Frontend 1. glass-card is a phantom class -- only .glass / .glass-strong / .glass-thin / .glass-ultra exist. All 4 uses resolved to NO backdrop / border / shadow -- the whole page rendered flat. Fixed to canonical 'glass rounded-2xl border border-border'. 2. h-full on bucket cards for equal heights (Motion wrapper was breaking grid stretch). 3. Custom-range popover redone as a fixed modal matching ConfirmDialog / AuthModal / ProfileModal / CommandPalette -- the app's canonical overlay pattern. Popover-inside-sticky-header was the wrong shape; pushed layout wide + got clipped. 4. Active pill fill: green -> --overlay-5. Matches every other tab bar in the app. Green is the semantic income/savings color and shouldn't leak into generic 'selected' state. 5. Divider tokens: border-[var(--overlay-5)] -> border-border (overlay-5 is a raised-surface token, not a hairline). Icon tile bg: overlay-5 -> overlay-3. 6. KPI hero scale: text-3xl -> text-2xl (matches MetricCard). 7. Dropped the overflow-x-hidden hack -- DataTable's default overflow-x-auto handles wide labels correctly. ## Backend 8. Education classifier bug: _classify_category used 'cat_lower in essential_set' -- EXACT string match. So a txn with category='Education & Learning' failed to match the default keyword 'education'. Same for 'Health & Insurance', 'Home Loan / EMI', 'Food & Dining' etc. Switched to word-boundary matching via _matches_investment_pattern. 9. User overrides ADD to defaults instead of replacing them. `user_essentials or defaults` silently dropped defaults when the user tagged their first custom category. Fixed to set union. ## Tests Added test_compound_category_matches_default_needs_via_word_boundary for Education / Health & Insurance case. Renamed and rewrote the override test to verify add-not-replace semantics. 287 backend tests pass (+2). 266 frontend tests pass, type-check + lint clean.
…ncated amounts) Screenshot showed amounts chopped to '₹12,91' / '₹5,71' / '₹3,13' and the 'Avg / mo' header truncated to 'Avg /'. Root cause traced: - DataTable renders `<td>` with NO `whitespace-nowrap` and the parent `<table>` has NO `table-layout: fixed`. widthClass (w-28) on the amount column is a hint, not a cap. - Three DataTables inside a `lg:grid-cols-3` grid each get ~33% of the viewport. Amount column loses the width fight against the Category cell (which holds the long "sub · sub · sub" line) and the browser truncates the tabular-numeric digits. - Two other analytics pages in the app (`CategoryBreakdown.tsx`, `TopMerchants.tsx`) solve the EXACT same problem with hand-rolled flex rows -- because DataTable is optimized for a full-width sortable table, not for cramming into a 33%-wide grid cell. ## Rewrite Replaced the whole DataTable + column-config block with the app's canonical Category+Amount row pattern from CategoryBreakdown.tsx: flex items-center gap-2 py-2 px-3 rounded-lg ├─ flex-1 min-w-0 (name block, truncates cleanly) │ ├─ text-sm font-medium truncate (category name) │ └─ text-[11px] truncate (top-3 subs joined with ·) └─ shrink-0 w-24 text-right text-sm tabular-nums whitespace-nowrap (amount) The critical fix is `shrink-0 w-24 whitespace-nowrap` on the amount span -- flexbox now respects the fixed width and never squeezes the number. Truncation happens on the name/sub side where it's readable. ## Also dropped - The `%` column (YNAB / Monarch / Copilot all drop % from category rows -- the ratio math lives in the bucket header cards). Removes two competing right-edges. - The sort-header UI (rows already sorted by amount desc from the backend; users don't sort inside a 10-row bucket column). - The overflow-x-hidden hack (no longer needed -- flex rows can't scroll horizontally by construction). ## Kept - Top-10 + expandable "Other (N more)" rollup for long-tail categories. - All existing behavior on bucket totals, sub-list rendering, and design-system tokens (glass rounded-2xl border border-border). 287 backend tests pass, 266 frontend tests pass, type-check + lint clean.
…Container Consistency wave, follow-up to the 50/30/20 rewrite. New primitive: - <Money> in components/ui codifies the canonical amount-cell rules (shrink-0 text-right tabular-nums whitespace-nowrap font-medium) so future callers can't drop whitespace-nowrap and re-hit the "₹12,91" truncation seen on /budget. Presets: sm/md/lg/xl width, bold, muted. Migrated to <Money>: - pages/budget/components/CategoryTable (amount + bucket avg) - components/analytics/CategoryBreakdown (category total + subcategory rows) PageContainer migrations (dropped hand-rolled min-h-dvh + max-w-7xl scaffold): - AnomalyReviewPage, FIRECalculatorPage, TransactionsPage, MorePage, SubscriptionTrackerPage. Palette drift fixes: - year-in-review/types.ts heatmapColors: slate rgba -> rawColors.app.red / app.green with hex+alpha (theme-aware). - constants/colors.ts: new rawColors.onAccent token so buttons on accent fills stay white in both themes without hard-coding '#fff'. - period-comparison/PeriodSelectors: replaced '#fff' with rawColors.onAccent. - shared/CommandPalette: replaced inline rgba boxShadow with the shared var(--glass-shadow-ultra) token + app-blue ambient glow. Verified: type-check green, eslint clean, 266/266 vitest tests pass.
Charts across the app were emitting rows past today's date, producing a flat-zero tail on line/area/bar series once we crossed into a new month. Two-site fix, both driven by an audit across pages, hooks and endpoints: Root cause -- getAnalyticsDateRange in lib/dateUtils.ts: - FY, yearly and monthly view modes returned end_date = the calendar end of the window (e.g. FY26 running 2026-04-01 to 2027-03-31), even when the window straddles "now". Any downstream aggregation over that range emits zero rows for the future portion. - New `capEndDateAtToday()` clamps end_date at today (all_time's null stays null). Applied inside the switch statement so every analytics hook consuming this range function (useAnalyticsTimeFilter, useDashboardMetrics, all TanStack Query hooks that pass start/end to the analytics endpoints) inherits the fix -- no per-caller change. Second bug -- year-in-review monthly bar chart: - MONTHS_SHORT.map iterates all 12 months regardless of what's actually reachable in the current calendar/fiscal year, so the Monthly Breakdown chart showed Sep..Dec (or Oct..Mar for FY) as zero bars. - Sliced by a `cutoff` computed against `new Date()`: full 12 for past years, current-month + 1 for current calendar year, elapsed-months + 1 for current FY (with the mod-arithmetic that handles fiscal wrap). Also exported `capSeriesToToday<T>(rows, key)` for any future caller that holds pre-computed month/day-keyed series in-memory -- generic, ISO-string comparison, handles Date-valued keys too. Not applied anywhere yet. Projection pages (FIRE, tax multi-year, retirement) intentionally untouched -- they build their own future ranges and never call getAnalyticsDateRange. Verified: type-check green, eslint clean, 275/275 vitest tests pass (9 new tests covering both helpers: today/yesterday/tomorrow/next-month boundaries, empty/mixed-order rows, Date-valued keys, null preservation).
- CHANGELOG.md: add Unreleased section for feat/comprehensive-hardening (PR #202) covering the 50/30/20 page, the Money primitive, the chart-cap helpers, DB-level cascade, security hardening, and design-system fixes. - CLAUDE.md: mention Money in the primitives list; add two new design-system bullets -- one for the Money rule ("amount cells use <Money>") and one for historical charts ("time-series data must not extend past today"). - CLAUDE.md DataTable bullet: warn that at narrow widths td truncates money and callers should reach for <Money> or a hand-rolled flex row instead.
Verified: frontend 275/275, backend 287/287, type-check + eslint + ruff clean. CRITICAL (4): - CategoryTable: drop unused `incomeTotal` prop (was silenced with `void`, which S3735 flags). Trimmed the sole caller. - spending_rule: extract `_MUTUAL_FUNDS_LABEL = "Mutual Funds"` constant -- literal was repeated 6x in `_TRANSFER_RELABEL_BY_ACCOUNT` (S1192). - spending_rule: extract `_aggregate_txns()` helper so `get_spending_rule_breakdown` drops from cognitive complexity 26 -> ~10 (S3776). - anomalies: extract `_grade_month()` staticmethod, dropping `_detect_high_expense_months` from 20 -> ~12 (S3776). MAJOR (5): - cascade migration: extract `_reflect_current_fk_names()` + hoist `_NAMING_CONVENTION`; `_upgrade_sqlite` drops 17 -> ~11 (S3776). - BudgetPage: extract `renderBody()` helper, replacing the loading / bad-shape / content nested ternary in JSX (S3358). - BucketCard: hoist `deltaSign` out of the nested `?:` inside a template literal (S3358). - PeriodPicker: wrap From / To label text in `<span>` so JSX children aren't ambiguously spaced against the `<input>` sibling (S6772 x2). Test files (5): - test_encryption: hoist base64 encode outside `pytest.raises` so only the decrypt call is inside the block (S5778). - test_spending_rule: split the `-- reason` off the `# noqa: N806` line into a preceding comment; Sonar's S7632 is strict about noqa syntax. - test_rate_limit_user_key: hoist the 4 synthetic test IPs into module constants with `# NOSONAR` (documented as test-only fakes, not leaked prod IPs) so S1313 stops firing on every literal use (7 issues -> 0).
…rdening # Conflicts: # CLAUDE.md
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Distilled from a full-repo audit that ran six research agents in parallel (transcribed to docs/plans/comprehensive-hardening-2026-07-04.md). Two live 404s fixed, six structural hardenings landed, the
/budgetspage replaced with a proper 50/30/20 Budget Rule view, and two real bugs caught + fixed during local E2E testing before push.What is in this branch (12 commits)
Live bug fixes
/ai-config/keywhile backend served/ai-config/key/reveal. Aligned./api/rates/instrumentswhilerates.pymounted at/rates. Also normalizedexchange-rates+stock-priceto/api/*so all three now pick up theno-storecache middleware they were silently bypassing.Security
LEDGER_SYNC_ENCRYPTION_KEYenv var, v2 ciphertext format with HKDF-SHA256 (RFC 5869, the correct primitive when the input is already a key). Legacy v1 blobs decrypt on next read and get re-encrypted as v2 in-band. Skipped Argon2id and the OWASP-600k PBKDF2 bump -- both are for password stretching, not KEK derivation.User.token_version--tvclaim baked into every JWT, verified inget_current_user. Bumped on logout / account reset. Previouslogoutendpoint was cosmetic (docstring literally admitted "access token remains technically valid until expiry"); now it kills every outstanding token in one integer write. Soft-accept legacy tokens during 7-day rollout, strict cutoff viaLEDGER_SYNC_JWT_STRICT_TV=trueon day 8.user_limiterkeyed on JWTsub, stacked with the existing IP-keyedlimiteron/api/uploadand/api/ai/bedrock/chat. Fixes the CGNAT/Vercel-edge shared-bucket problem.Database
ondelete=CASCADEbackfilled on 20 user_id FKs (was 2/21 per the audit) -- raw-SQL user delete no longer orphans children. Dialect-aware Alembic migration: Postgres discovers constraint names viainformation_schema; SQLite usesbatch_alter_tablewith a naming-convention pass to handle both named and anonymous FKs cleanly.Analytics correctness
\btax(es)?\bnow catchesGST,TDS,cess,surcharge,advance tax,self-assessment. Word boundaries still excludeTaxi/Syntax.anomaly_expense_threshold(stdev multiplier default 2.0) is anchor-preserved to the new modified-Z cutoff (3.5 default) so nobody sees a semantic drift on deploy day:effective_z = 3.5 * (stored / 2.0).CI + hygiene
>=3.11/ mypy 3.12 / ruff py313 / CI 3.13 / migrate 3.14. All four cleanly on 3.13 now.timeout-minuteson every workflow job -- none had one previously; a hunguv synccould burn 6 hours of runner time.migrate.ymlpaths trigger fixed -- addeddb/_models/**since real model files live there now, not just the facade.Makefiledeleted (referenced Poetry + black; project uses uv + ruff)..github/renovate.jsondeleted (rootrenovate.jsonextending the shared preset was already the live config).New
/budgetspage (rip-and-replace)GET /api/analytics/v2/spending-rule?start_date=&end_date=returning{period, income_total, expense_total, savings_amount, targets, buckets, categories}. Bucket classification readsessential_categories+investment_account_mappingsprefs, falls back to opinionated Indian defaults (Rent, EMI, Utilities, Groceries, Food and Dining, Fuel, Insurance, Healthcare, Family Support, Internet, Phone for Needs; SIP, PPF, EPF, NPS, MF, Stocks, ELSS, RD, SSY + brokers for Savings).useBudgethook +types.ts+ 4 sub-components (AddBudgetForm,BudgetCharts,BudgetDefaultsPanel,BudgetRowItem). Sidebar label "Budget Manager" -> "Budget Rule".Bugs caught + fixed during local E2E (commit
ff6b07c)create_foreign_keywithout a matchingdrop_constraint; on SQLitebatch_alter_tabletreats that as APPEND, not REPLACE. Every table ended up with 2 FKs to users. Rewrote to reflect existing FK names first (using alembic naming_convention to handle anonymous FKs), then drop+create in the same batch./budgetspage crashed in demo mode -- axios demo adapter had a catch-all for/analytics/v2/*returning{data: [], count: 0}, wrong shape for the new endpoint. Added a proper demo stub with a zero-state response + a defensive shape check inBudgetPage.Tests
alembic upgrade headruns cleanly against the dev SQLite DB, all 19 tables have exactly 1 CASCADE FK to users, backend/health/dbreturns{status: "ok", database: "connected"}, endpoint returns 401 for unauth (correct).Rollout notes
Ordered dependencies:
LEDGER_SYNC_ENCRYPTION_KEYunset -- app falls back tojwt_secret_keywith a deprecation warning. Zero behavioral change.LEDGER_SYNC_ENCRYPTION_KEYon Vercel:python -c 'import secrets; print(secrets.token_urlsafe(32))'. New writes use v2; old v1 blobs get transparently upgraded on next reveal.LEDGER_SYNC_JWT_STRICT_TV=trueon Vercel. Any surviving pre-migration token now fails; user re-logs in once (refresh TTL is 7d so this is a non-event for active users)._decrypt_v1branch and thejwt_secret_keyfallback inencryption.py.What is NOT in this branch (intentionally deferred)
# CLAUDE.mdduplicate heading, page count reconciliation).