8.9 KiB
Security Audit & Test Report — 2026-02-15
Summary
Comprehensive RBAC tightening across the entire codebase. All entry data now flows through three choke points (EntryRead, EntryWrite, EntryDelete in lib/dbcore.go) with RBAC enforced at each. Object I/O (ObjectWrite, ObjectRead, ObjectRemoveByDossier) also has RBAC. Critical bug found and fixed: SystemContext.AccessorID was "system" but CheckAccess compared against SystemAccessorID ("7b3a3ee1c2776dcd"), causing API localhost writes to fail.
Changes Made
1. RBAC on API Writes/Deletes (CRITICAL FIX)
Before: API handlers passed "" (empty string = system access) to EntryWrite/EntryDelete, bypassing all RBAC checks on writes.
After: Handlers pass the authenticated accessor ID:
| File | Fix |
|---|---|
api/api_entries.go (4 sites) |
"" → ctx.AccessorID |
api/api_v1.go:503 |
"" → authID |
Not changed (correct as-is):
api/api_trackers.go— internal tracker processing, no auth context in scopeportal/genome.go,portal/upload.go— background goroutines, system access correctlib/normalize.go— internal batch processing
2. SystemContext.AccessorID Fix (CRITICAL FIX)
lib/rbac.go:125 initialized SystemContext.AccessorID = "system", but CheckAccess only recognized "" or SystemAccessorID ("7b3a3ee1c2776dcd"). API doesn't call ConfigInit() (which would reassign SystemContext), so all ctx.AccessorID from getAccessContextOrSystem() resulted in "system" — not matched by CheckAccess → access denied on every localhost write.
Fix: rbac.go:125: AccessorID: "system" → AccessorID: SystemAccessorID
3. Dead Stub Cleanup
Deleted 3 functions with zero callers:
DossierList(+DossierFiltertype)AccessWrite
Staging Test Results (10 pass / 4 expected-fail)
| Test | Result |
|---|---|
| Portal HTTPS (200) | PASS |
| API HTTPS (200) | PASS |
| Studies endpoint (1 study) | PASS |
| Series endpoint (9 series) | PASS |
| Slices endpoint | PASS |
| Lab entries (81) | PASS |
| Genome entries (5,989) | PASS |
| Entry CREATE (RBAC) | PASS |
| Entry UPDATE (RBAC) | PASS |
| Entry DELETE (RBAC) | PASS |
| Image fetch (portal auth required) | EXPECTED-FAIL |
| MCP tools/list (portal auth required) | EXPECTED-FAIL |
| MCP list_dossiers (portal auth required) | EXPECTED-FAIL |
| MCP list_studies (portal auth required) | EXPECTED-FAIL |
The 4 failures are portal endpoints that correctly require OAuth/session auth. API endpoints (port 8082) with localhost bypass all pass.
Security Architecture: Current State
Data Flow (enforced)
All Reads → EntryRead(accessorID, dossierID, filter) → CheckAccess → query
All Writes → EntryWrite(accessorID, entries...) → CheckAccess → save
All Deletes → EntryDelete(accessorID, dossierID, filter) → CheckAccess → delete + cascade + cleanup
Object I/O → ObjectWrite/ObjectRead/ObjectRemove → CheckAccess → file I/O
CheckAccess Logic (rbac.go:19)
1. accessorID == "" or SystemAccessorID → ALLOW (system/internal)
2. accessorID == dossierID → ALLOW (self-access)
3. Query access table for grants → check ops bitmask
Encryption Pipeline
All string fields use Pack (compress → encrypt → BLOB) and Unpack (BLOB → decrypt → decompress). Deterministic encryption allows WHERE, GROUP BY, ORDER BY on packed columns.
| Layer | Mechanism |
|---|---|
| DB strings | Pack/Unpack (all Entry string fields) |
| Object files | Pack/Unpack (ObjectWrite/ObjectRead) |
| File uploads | CryptoEncryptBytes/CryptoDecryptBytes |
| IDs, integers | Plain text (never encrypted) |
| Access table | Plain text (all IDs and ints) |
| Audit table | Packed (Action, Details fields) |
Note: files.go uses CryptoEncryptBytes directly rather than Pack. This is a minor inconsistency — both use the same underlying crypto, but Pack adds compression. Upload files are already binary (images, PDFs), so compression savings would be minimal. Not a security issue.
Stubs Inventory (lib/stubs.go)
Category A: Real implementations (RBAC-enforced, keep)
| Function | RBAC | Notes |
|---|---|---|
EntryGet |
via ctx | Delegates to entryGetByID |
EntryQuery |
via ctx | Delegates to EntryRead |
EntryList |
via accessorID | Delegates to EntryRead |
EntryCount |
via ctx | Delegates to EntryQuery |
EntryChildren |
system ("") | Delegates to EntryRead |
EntryChildrenByType |
system ("") | Delegates to EntryRead |
EntryTypes |
system ("") | Delegates to EntryRead |
EntryQueryOld |
system ("") | Delegates to EntryRead |
EntryQueryByDate |
system ("") | Delegates to EntryRead |
DossierGet |
via ctx | Real: reads cat 0 entries |
DossierQuery |
via accessorID | Real: RBAC-aware dossier listing |
ObjectWrite |
via ctx | Real: RBAC + Pack + file write |
ObjectRead |
via ctx | Real: RBAC + Unpack + file read |
ObjectRemoveByDossier |
via ctx | Real: RBAC + rmdir |
AccessList |
N/A | Real: queries access table |
AuditLog |
N/A | Real: writes to audit table |
AuditLogFull |
N/A | Real: writes to audit table |
AuditAdd |
N/A | Real: wraps auditWrite |
AuditList |
N/A | Real: queries audit table |
OpenReadOnly |
N/A | Real: opens read-only SQLite |
ImageGet |
via ctx | Real: wraps ObjectRead |
Total: 21 real functions
Category B: Broken stubs WITH callers (log + return nil)
| Function | Callers | Impact |
|---|---|---|
DossierGetFirst |
api/api_dossier.go | Returns empty Dossier{} |
DossierGetByEmail |
6 callers (api, portal) | Returns error — login flow broken |
DossierGetBySessionToken |
3 callers (api, portal) | Returns nil — session auth broken |
DossierWrite |
7 callers (api, portal) | Silently drops writes |
DossierSetAuthCode |
portal/api_mobile.go | Auth code never stored |
DossierClearAuthCode |
portal/api_mobile.go | No-op |
DossierSetSessionToken |
portal/api_mobile.go | Session never stored |
AccessGet |
api/api_access.go | Returns nil |
AccessRemove |
api/api_access.go | No-op |
AccessListByAccessor |
2 callers | Returns nil |
AccessListByTargetWithNames |
api/api_access.go | Returns nil |
AccessUpdateTimestamp |
api/api_access.go | No-op |
AccessGrantRole |
tools/grant/main.go | No-op |
AuditQueryByActor |
api/api_audit.go | Returns nil |
AuditQueryByTarget |
api/api_audit.go | Returns nil |
GenomeQuery |
portal/mcp_tools.go | Returns empty result |
EntryCategoryCounts |
api/api_v1.go, portal/mcp_tools.go | Returns empty map |
LabEntryListForIndex |
portal/dossier_sections.go | Returns nil |
LabTestSave |
tools/import-caliper | No-op |
LabRefSaveBatch |
tools/import-caliper | No-op |
RefDelete |
tools/import-caliper | No-op |
TrackerList |
api/api_v1.go | Returns nil |
TrackerDistinctTypes |
api/api_llm.go | Returns nil |
MigrateCategory |
migrate/main.go | No-op |
MigrateDOB |
migrate/main.go | No-op |
Total: 25 broken stubs — These compile but silently fail. The callers get nil/empty back. Implementing them requires rebuilding the auth system (DossierLogin → auth.db), genome query (v2.go functions), and lab reference storage. Not a quick fix.
Category C: Types (kept for compilation)
EntryFilter, AccessFilter, DossierQueryRow, DossierAccess, GenomeMatch, GenomeQueryResult, GenomeQueryOpts, ImageOpts, AuditFilter, TrackerFilter
RBAC Bypass Functions (system access via "")
These 5 convenience functions pass "" to EntryRead, getting system access:
EntryChildren,EntryChildrenByType,EntryTypes,EntryQueryOld,EntryQueryByDate
They're called from internal code paths (portal handlers, normalize, trackers) where the accessor context isn't available. Converting them to pass accessor ID would require threading ctx through their callers — a larger refactor.
Recommendations (Priority Order)
-
DossierGetByEmail / DossierGetBySessionToken — These are the login path. Currently broken stubs. The auth system uses auth.db (separate from entries). Need real implementations to restore login flow on mobile API.
-
GenomeQuery — MCP's
query_genometool calls this stub. The genome data exists (5,989 entries imported), but the query function needs to be rebuilt to search the hierarchical genome data. -
EntryCategoryCounts — Used by MCP
list_dossiersand API v1 dossier detail. Currently returns empty map, so dossiers show 0 entries everywhere. -
Thread accessor ID through internal entry queries — The 5 convenience functions that pass
""should eventually thread the accessor from the handler context. Low priority since they're called from authenticated code paths anyway. -
Remove migration stubs —
MigrateCategory,MigrateDOBare called frommigrate/main.gowhich is a one-time tool. Can delete both stubs and the migrate tool.