Skip to content

Commit 62b74db

Browse files
author
amar-python
committed
feat: security + frontend improvements — hide DB details from health; fix T&E table drop guard; frontend updates
1 parent 6bcb215 commit 62b74db

33 files changed

Lines changed: 2214 additions & 274 deletions

.coverage

52 KB
Binary file not shown.

README.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ Get-Content env.dev.example | ForEach-Object {
119119
}
120120
121121
# 4. Provision dev database (Git Bash)
122-
& "C:\Program Files\Git\bin\bash.exe" -c "PGUSER=postgres PGHOST=localhost PGPORT=5433 PGPASSWORD=devpassword123 bash scripts/provision_full_test_env.sh"
122+
& "C:\Program Files\Git\bin\bash.exe" -c "PGUSER=postgres PGHOST=localhost PGPORT=5433 PGPASSWORD=changeme_local_only bash scripts/provision_full_test_env.sh"
123123
124124
# 5. Terminal 1 — start the API
125125
.\scripts\start-api.ps1
@@ -143,7 +143,7 @@ Start-Process "http://localhost:8080"
143143
| `GET` | `/api/csv/files` | List all uploaded CSVs |
144144
| `GET` | `/api/csv/tables/{name}/rows` | Preview rows of a dynamic table |
145145
| `DELETE` | `/api/csv/files/{id}` | Drop a dynamic table (requires `API_ALLOW_DESTRUCTIVE=true`) |
146-
| `GET` | `/api/audit/log` | Paginated deletion history (who/what/when was dropped) |
146+
| `GET` | `/api/audit/log` | Paginated deletion history (who/what/when was dropped) — not currently called by the shipped frontend, see note below |
147147
| `GET` | `/api/te/tables` | Existence + row counts for the 12 fixed T&E tables |
148148

149149
Interactive docs: `http://localhost:8000/docs`
@@ -152,6 +152,16 @@ Interactive docs: `http://localhost:8000/docs`
152152

153153
## Frontend Features (13)
154154

155+
> **Naming note:** the "audit log" features below (`/audit` route, `audit.tsx`)
156+
> operate on **this browser's local import history** (`localStorage`, populated
157+
> as you run imports in this session) — not the backend's `GET /api/audit/log`
158+
> endpoint (server-side deletion history), which the shipped frontend never
159+
> calls. `audit.tsx` also renders a small, separate, hardcoded developer
160+
> changelog widget (`src/lib/audit-log.ts`) alongside the import-history view;
161+
> that's app release notes, unrelated to either audit trail above. Three
162+
> different things share the word "audit" here — worth knowing before you go
163+
> looking for server-side audit data in the UI.
164+
155165
| Feature | Description |
156166
|---|---|
157167
| Audit log filters | Filter by date, status, file name |

api/main.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,8 @@ def health() -> dict:
103103
"uploads_schema": settings.UPLOADS_SCHEMA,
104104
}
105105
except Exception as exc: # noqa: BLE001 — surface DB reachability to the UI
106+
# Log the real exception server-side, but don't return it: /api/health
107+
# is deliberately unauthenticated (see BUG-035 comment above), and
108+
# psycopg2's OperationalError text can include host/port/user details.
106109
logger.warning("Health check failed: %s", exc)
107-
return {"status": "degraded", "error": str(exc).split("\n")[0][:200]}
110+
return {"status": "degraded", "error": "Database is unreachable or not fully bootstrapped."}

api/requirements.txt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
fastapi>=0.140.7
2-
uvicorn[standard]>=0.51.0
3-
psycopg2-binary>=2.9.12
4-
python-multipart>=0.0.9
5-
pydantic>=2.13.4
1+
fastapi>=0.140.7,<0.142.0
2+
uvicorn[standard]>=0.51.0,<0.53.0
3+
psycopg2-binary>=2.9.12,<3.0.0
4+
python-multipart>=0.0.9,<0.1.0
5+
pydantic>=2.13.4,<3.0.0
66
# BUG-032: schema migrations for csv_uploads.* — auto-runs at API startup
77
# via alembic upgrade head (see api/db.py bootstrap and alembic/ directory).
8-
alembic>=1.13.0
8+
alembic>=1.13.0,<2.0.0

api/routers/csv_routes.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
from __future__ import annotations
44

5+
import logging
6+
57
from fastapi import APIRouter, Depends, HTTPException
68
from psycopg2 import sql
79
from pydantic import BaseModel, Field
@@ -13,6 +15,8 @@
1315
from api.services.dynamic_loader import upload_dynamic
1416
from api.services.te_loader import match_te_table, upload_te
1517

18+
logger = logging.getLogger(__name__)
19+
1620
# BUG-035: auth is applied here (per-router) instead of at app level in
1721
# api/main.py, so /api/health can stay unauthenticated for external monitoring.
1822
router = APIRouter(
@@ -46,10 +50,11 @@ def preview(req: PreviewRequest) -> dict:
4650
try:
4751
result = build_preview(req.content)
4852
except Exception as exc: # noqa: BLE001 — surface any parser failure as structured JSON
53+
logger.warning("CSV preview parse failed for %r: %s", req.fileName, exc)
4954
return {
5055
"status": "invalid_structure",
5156
"reason": "parse_failed",
52-
"message": f"The CSV couldn't be parsed: {str(exc)[:200]}",
57+
"message": "The CSV couldn't be parsed. Check the file's structure and try again.",
5358
}
5459
if result.get("status") == "ok":
5560
# Suggest a T&E table if the columns fit one (drives the mode picker in the UI)

api/services/dynamic_loader.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ def _do_upload(
134134
_log(logs, "duplicate_check", "Checking existing filename")
135135
cur.execute(
136136
sql.SQL(
137-
"SELECT id, file_name, table_name, file_hash, row_count FROM {}.csv_files WHERE file_name = %s"
137+
"SELECT id, file_name, table_name, file_hash, row_count, mode "
138+
"FROM {}.csv_files WHERE file_name = %s"
138139
).format(sql.Identifier(schema)),
139140
(file_name,),
140141
)
@@ -151,11 +152,16 @@ def _do_upload(
151152
"logs": logs,
152153
}
153154
_log(logs, "overwrite", f'Overwriting previous upload "{name_match[1]}"', "warn")
154-
cur.execute(
155-
sql.SQL("DROP TABLE IF EXISTS {}.{}").format(
156-
sql.Identifier(schema), sql.Identifier(name_match[2])
155+
# table_name is only a bare identifier owned by this schema for
156+
# dynamic-mode rows (csv_<hash>); TE-mode rows store "schema.table"
157+
# pointing at a shared T&E table that must never be dropped here.
158+
# Same guard as te_loader._cleanup_prior_registry_entries.
159+
if name_match[5] == "dynamic" and name_match[2].startswith("csv_"):
160+
cur.execute(
161+
sql.SQL("DROP TABLE IF EXISTS {}.{}").format(
162+
sql.Identifier(schema), sql.Identifier(name_match[2])
163+
)
157164
)
158-
)
159165
cur.execute(
160166
sql.SQL("DELETE FROM {}.csv_files WHERE id = %s").format(sql.Identifier(schema)),
161167
(name_match[0],),
@@ -166,7 +172,8 @@ def _do_upload(
166172
_log(logs, "duplicate_check", "Checking existing content hash")
167173
cur.execute(
168174
sql.SQL(
169-
"SELECT id, file_name, table_name, row_count FROM {}.csv_files WHERE file_hash = %s"
175+
"SELECT id, file_name, table_name, row_count, mode "
176+
"FROM {}.csv_files WHERE file_hash = %s"
170177
).format(sql.Identifier(schema)),
171178
(file_hash,),
172179
)
@@ -183,11 +190,13 @@ def _do_upload(
183190
"logs": logs,
184191
}
185192
_log(logs, "overwrite", f'Overwriting previous content match "{content_match[1]}"', "warn")
186-
cur.execute(
187-
sql.SQL("DROP TABLE IF EXISTS {}.{}").format(
188-
sql.Identifier(schema), sql.Identifier(content_match[2])
193+
# Same TE-vs-dynamic guard as above.
194+
if content_match[4] == "dynamic" and content_match[2].startswith("csv_"):
195+
cur.execute(
196+
sql.SQL("DROP TABLE IF EXISTS {}.{}").format(
197+
sql.Identifier(schema), sql.Identifier(content_match[2])
198+
)
189199
)
190-
)
191200
cur.execute(
192201
sql.SQL("DELETE FROM {}.csv_files WHERE id = %s").format(sql.Identifier(schema)),
193202
(content_match[0],),

build/csv/loader_mariadb.sh

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ def quote_ident(name):
6464
6565
cols = next(csv.reader([sys.argv[1]]))
6666
sanitized = [sanitize(c, i + 1) for i, c in enumerate(cols)]
67+
# Dedupe collisions after sanitisation (mirrors sanitize_columns() in
68+
# api/services/csv_parse.py) — otherwise two headers that sanitise to the
69+
# same name produce a CREATE TABLE with a duplicate column instead of a
70+
# clean rejection.
71+
assigned = set()
72+
counter = {}
73+
deduped = []
74+
for base in sanitized:
75+
name = base
76+
while name in assigned:
77+
counter[base] = counter.get(base, 0) + 1
78+
name = f'{base}_{counter[base] + 1}'
79+
assigned.add(name)
80+
deduped.append(name)
81+
sanitized = deduped
6782
bad = [c for c in sanitized if not re.match(r'^[a-z_][a-z0-9_]*\$', c)]
6883
if bad:
6984
sys.exit('unsafe column name(s) after sanitisation: ' + ', '.join(bad))
@@ -86,6 +101,21 @@ def quote_ident(name):
86101
87102
cols = next(csv.reader([sys.argv[1]]))
88103
sanitized = [sanitize(c, i + 1) for i, c in enumerate(cols)]
104+
# Dedupe collisions after sanitisation (mirrors sanitize_columns() in
105+
# api/services/csv_parse.py) — otherwise two headers that sanitise to the
106+
# same name produce a CREATE TABLE with a duplicate column instead of a
107+
# clean rejection.
108+
assigned = set()
109+
counter = {}
110+
deduped = []
111+
for base in sanitized:
112+
name = base
113+
while name in assigned:
114+
counter[base] = counter.get(base, 0) + 1
115+
name = f'{base}_{counter[base] + 1}'
116+
assigned.add(name)
117+
deduped.append(name)
118+
sanitized = deduped
89119
bad = [c for c in sanitized if not re.match(r'^[a-z_][a-z0-9_]*\$', c)]
90120
if bad:
91121
sys.exit('unsafe column name(s) after sanitisation: ' + ', '.join(bad))

build/csv/loader_postgresql.sh

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,21 @@ def quote_ident(name):
7474
7575
cols = next(csv.reader([sys.argv[1]]))
7676
sanitized = [sanitize(c, i + 1) for i, c in enumerate(cols)]
77+
# Dedupe collisions after sanitisation (mirrors sanitize_columns() in
78+
# api/services/csv_parse.py) — otherwise two headers that sanitise to the
79+
# same name produce a CREATE TABLE with a duplicate column instead of a
80+
# clean rejection.
81+
assigned = set()
82+
counter = {}
83+
deduped = []
84+
for base in sanitized:
85+
name = base
86+
while name in assigned:
87+
counter[base] = counter.get(base, 0) + 1
88+
name = f'{base}_{counter[base] + 1}'
89+
assigned.add(name)
90+
deduped.append(name)
91+
sanitized = deduped
7792
bad = [c for c in sanitized if not re.match(r'^[a-z_][a-z0-9_]*\$', c)]
7893
if bad:
7994
sys.exit('unsafe column name(s) after sanitisation: ' + ', '.join(bad))
@@ -100,6 +115,21 @@ def quote_ident(name):
100115
101116
cols = next(csv.reader([sys.argv[1]]))
102117
sanitized = [sanitize(c, i + 1) for i, c in enumerate(cols)]
118+
# Dedupe collisions after sanitisation (mirrors sanitize_columns() in
119+
# api/services/csv_parse.py) — otherwise two headers that sanitise to the
120+
# same name produce a CREATE TABLE with a duplicate column instead of a
121+
# clean rejection.
122+
assigned = set()
123+
counter = {}
124+
deduped = []
125+
for base in sanitized:
126+
name = base
127+
while name in assigned:
128+
counter[base] = counter.get(base, 0) + 1
129+
name = f'{base}_{counter[base] + 1}'
130+
assigned.add(name)
131+
deduped.append(name)
132+
sanitized = deduped
103133
bad = [c for c in sanitized if not re.match(r'^[a-z_][a-z0-9_]*\$', c)]
104134
if bad:
105135
sys.exit('unsafe column name(s) after sanitisation: ' + ', '.join(bad))

0 commit comments

Comments
 (0)