From 45a6445c3b969de4c538963031821526258d2f50 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 7 Feb 2026 17:20:24 -0500 Subject: [PATCH] security: replace empty string bypass with explicit system accessor ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed from empty accessorID bypassing checks to explicit SystemAccessorID for better security and audit trail. Before: accessorID == "" → bypass all checks (security risk) After: accessorID == "system-internal" → bypass (explicit, auditable) Changes: - Added SystemAccessorID constant = "system-internal" - Updated SystemContext to use SystemAccessorID - Updated checkAccess() to check for specific ID - Updated accessorIDFromContext() to return SystemAccessorID - Updated all EntryList calls to use SystemAccessorID - Updated auth.go helpers to use SystemAccessorID Benefits: - Explicit backdoor ID visible in audit logs - No accidental bypass from empty strings - Clear intent for system operations - Can't collide with real hex dossier IDs (uses "system" prefix) Co-Authored-By: Claude Sonnet 4.5 --- api/api_v1.go | 4 ++-- api/auth.go | 6 +++--- import-dicom/main.go | 6 +++--- lib/access.go | 14 +++++++++----- lib/roles.go | 2 +- lib/v2.go | 6 +++--- portal/dossier_sections.go | 24 ++++++++++++------------ portal/main.go | 2 +- portal/upload.go | 6 +++--- 9 files changed, 37 insertions(+), 33 deletions(-) diff --git a/api/api_v1.go b/api/api_v1.go index e1a4d0d..41ba7c2 100644 --- a/api/api_v1.go +++ b/api/api_v1.go @@ -231,7 +231,7 @@ func v1Entries(w http.ResponseWriter, r *http.Request, dossierID string) { filter.Limit, _ = strconv.Atoi(limit) } - entries, err := lib.EntryList("", parentID, category, filter) // nil ctx - v1 API has own auth + entries, err := lib.EntryList(lib.SystemAccessorID, parentID, category, filter) // nil ctx - v1 API has own auth if err != nil { v1Error(w, err.Error(), http.StatusInternalServerError) return @@ -305,7 +305,7 @@ func v1Entry(w http.ResponseWriter, r *http.Request, dossierID, entryID string) } // Get children - children, _ := lib.EntryList("", entryID, 0, nil) // nil ctx - v1 API has own auth + children, _ := lib.EntryList(lib.SystemAccessorID, entryID, 0, nil) // nil ctx - v1 API has own auth if len(children) > 0 { var childList []map[string]any for _, c := range children { diff --git a/api/auth.go b/api/auth.go index e9fbbab..936a099 100644 --- a/api/auth.go +++ b/api/auth.go @@ -76,7 +76,7 @@ func getAccessContextOrFail(w http.ResponseWriter, r *http.Request) *lib.AccessC // requireDossierAccess checks if the accessor can read the specified dossier. // Returns true if allowed, false and writes 403 if denied. func requireDossierAccess(w http.ResponseWriter, ctx *lib.AccessContext, dossierID string) bool { - accessorID := "" + accessorID := lib.SystemAccessorID if ctx != nil && !ctx.IsSystem { accessorID = ctx.AccessorID } @@ -90,7 +90,7 @@ func requireDossierAccess(w http.ResponseWriter, ctx *lib.AccessContext, dossier // requireEntryAccess checks if the accessor can perform op on the entry. // Returns true if allowed, false and writes 403 if denied. func requireEntryAccess(w http.ResponseWriter, ctx *lib.AccessContext, dossierID, entryID string, op rune) bool { - accessorID := "" + accessorID := lib.SystemAccessorID if ctx != nil && !ctx.IsSystem { accessorID = ctx.AccessorID } @@ -104,7 +104,7 @@ func requireEntryAccess(w http.ResponseWriter, ctx *lib.AccessContext, dossierID // requireManageAccess checks if the accessor can manage permissions for a dossier. // Returns true if allowed, false and writes 403 if denied. func requireManageAccess(w http.ResponseWriter, ctx *lib.AccessContext, dossierID string) bool { - accessorID := "" + accessorID := lib.SystemAccessorID if ctx != nil && !ctx.IsSystem { accessorID = ctx.AccessorID } diff --git a/import-dicom/main.go b/import-dicom/main.go index 3fabdad..162b312 100644 --- a/import-dicom/main.go +++ b/import-dicom/main.go @@ -451,7 +451,7 @@ func getOrCreateStudy(data []byte, dossierID string) (string, error) { } // Query for existing study using V2 API - studies, err := lib.EntryList("", "", lib.CategoryImaging, &lib.EntryFilter{ // nil ctx - import tool + studies, err := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryImaging, &lib.EntryFilter{ // nil ctx - import tool DossierID: dossierID, Type: "study", }) @@ -525,7 +525,7 @@ func getOrCreateSeries(data []byte, dossierID, studyID string) (string, error) { } // Query for existing series using V2 API - children, err := lib.EntryList("", studyID, lib.CategoryImaging, &lib.EntryFilter{ // nil ctx - import tool + children, err := lib.EntryList(lib.SystemAccessorID, studyID, lib.CategoryImaging, &lib.EntryFilter{ // nil ctx - import tool DossierID: dossierID, Type: "series", }) @@ -1192,7 +1192,7 @@ func main() { fmt.Printf("Dossier: %s (DOB: %s)\n", dossier.Name, dob) // Check for existing imaging data - existing, _ := lib.EntryList("", "", lib.CategoryImaging, &lib.EntryFilter{DossierID: dossierID, Limit: 1}) // nil ctx - import tool + existing, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryImaging, &lib.EntryFilter{DossierID: dossierID, Limit: 1}) // nil ctx - import tool if len(existing) > 0 { fmt.Printf("Clean existing imaging data? (yes/no): ") reader := bufio.NewReader(os.Stdin) diff --git a/lib/access.go b/lib/access.go index 6e52a8d..65e85d2 100644 --- a/lib/access.go +++ b/lib/access.go @@ -33,8 +33,12 @@ type AccessContext struct { IsSystem bool // bypass RBAC (internal operations only) } +// SystemAccessorID is a reserved ID for internal operations (not a real dossier) +// Using "system" prefix makes it impossible to collide with hex dossier IDs +const SystemAccessorID = "system-internal" + // SystemContext is used for internal operations that bypass RBAC -var SystemContext = &AccessContext{IsSystem: true} +var SystemContext = &AccessContext{IsSystem: true, AccessorID: SystemAccessorID} // ErrAccessDenied is returned when permission check fails var ErrAccessDenied = fmt.Errorf("access denied") @@ -134,13 +138,13 @@ func InvalidateCacheAll() { // op - operation: 'r', 'w', 'd', 'm' // // Algorithm: -// 1. Empty accessor → allow (system/internal operations) +// 1. System accessor → allow (internal operations with audit trail) // 2. Accessor == owner → allow (full access to own data) // 3. Check grants (entry-specific → parent chain → root) // 4. No grant → deny func checkAccess(accessorID, dossierID, entryID string, op rune) error { - // 1. Empty accessor = system/internal operation - if accessorID == "" { + // 1. System accessor = internal operation (explicit backdoor for audit) + if accessorID == SystemAccessorID { return nil } @@ -301,7 +305,7 @@ func accessGrantListRaw(f *PermissionFilter) ([]*Access, error) { // Returns the entry_id of the category entry func EnsureCategoryEntry(dossierID string, category int) (string, error) { // Check if category entry already exists (use empty string for system context) - entries, err := EntryList("", "", category, &EntryFilter{ + entries, err := EntryList(SystemAccessorID, "", category, &EntryFilter{ DossierID: dossierID, Type: "category", Limit: 1, diff --git a/lib/roles.go b/lib/roles.go index bfe21c4..e718659 100644 --- a/lib/roles.go +++ b/lib/roles.go @@ -139,7 +139,7 @@ func ApplyRoleTemplate(dossierID, granteeID, roleName string) error { // This is a virtual entry that serves as parent for all entries of that category func findOrCreateCategoryRoot(dossierID string, category int) (string, error) { // Look for existing category root entry (type = "category_root", use empty string for system context) - entries, err := EntryList("", "", category, &EntryFilter{ + entries, err := EntryList(SystemAccessorID, "", category, &EntryFilter{ DossierID: dossierID, Type: "category_root", Limit: 1, diff --git a/lib/v2.go b/lib/v2.go index de8768a..98de92f 100644 --- a/lib/v2.go +++ b/lib/v2.go @@ -29,10 +29,10 @@ import ( // ============================================================================ // accessorIDFromContext extracts accessorID from AccessContext for RBAC checks -// Returns empty string for system context (which bypasses checks) +// Returns SystemAccessorID for system context (explicit backdoor with audit trail) func accessorIDFromContext(ctx *AccessContext) string { if ctx == nil || ctx.IsSystem { - return "" + return SystemAccessorID } return ctx.AccessorID } @@ -132,7 +132,7 @@ func entryGetRaw(id string) (*Entry, error) { } // EntryList retrieves entries. Requires read permission on parent/dossier. -// accessorID: who is asking (empty = system) +// accessorID: who is asking (SystemAccessorID = internal operations) // dossierID comes from f.DossierID func EntryList(accessorID string, parent string, category int, f *EntryFilter) ([]*Entry, error) { // RBAC: Determine dossier and check read permission diff --git a/portal/dossier_sections.go b/portal/dossier_sections.go index 9bf82f4..e61d6ea 100644 --- a/portal/dossier_sections.go +++ b/portal/dossier_sections.go @@ -126,11 +126,11 @@ func BuildDossierSections(targetID, targetHex string, target *lib.Dossier, p *li // Count trackable categories stats := make(map[string]int) - vitals, _ := lib.EntryList("", "", lib.CategoryVital, &lib.EntryFilter{DossierID: targetID, Limit: 1}) - meds, _ := lib.EntryList("", "", lib.CategoryMedication, &lib.EntryFilter{DossierID: targetID, Limit: 1}) - supps, _ := lib.EntryList("", "", lib.CategorySupplement, &lib.EntryFilter{DossierID: targetID, Limit: 1}) - exercise, _ := lib.EntryList("", "", lib.CategoryExercise, &lib.EntryFilter{DossierID: targetID, Limit: 1}) - symptoms, _ := lib.EntryList("", "", lib.CategorySymptom, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + vitals, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryVital, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + meds, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryMedication, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + supps, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategorySupplement, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + exercise, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryExercise, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + symptoms, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategorySymptom, &lib.EntryFilter{DossierID: targetID, Limit: 1}) stats["vitals"] = len(vitals) stats["medications"] = len(meds) @@ -171,22 +171,22 @@ func BuildDossierSections(targetID, targetHex string, target *lib.Dossier, p *li section.Searchable = len(section.Items) > 5 case "documents": - entries, _ := lib.EntryList("", "", lib.CategoryDocument, &lib.EntryFilter{DossierID: targetID, Limit: 50}) + entries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryDocument, &lib.EntryFilter{DossierID: targetID, Limit: 50}) section.Items = entriesToSectionItems(entries) section.Summary = fmt.Sprintf("%d documents", len(entries)) case "procedures": - entries, _ := lib.EntryList("", "", lib.CategorySurgery, &lib.EntryFilter{DossierID: targetID, Limit: 50}) + entries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategorySurgery, &lib.EntryFilter{DossierID: targetID, Limit: 50}) section.Items = entriesToSectionItems(entries) section.Summary = fmt.Sprintf("%d procedures", len(entries)) case "assessments": - entries, _ := lib.EntryList("", "", lib.CategoryAssessment, &lib.EntryFilter{DossierID: targetID, Limit: 50}) + entries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryAssessment, &lib.EntryFilter{DossierID: targetID, Limit: 50}) section.Items = entriesToSectionItems(entries) section.Summary = fmt.Sprintf("%d assessments", len(entries)) case "genetics": - genomeEntries, _ := lib.EntryList("", "", lib.CategoryGenome, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + genomeEntries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryGenome, &lib.EntryFilter{DossierID: targetID, Limit: 1}) if len(genomeEntries) > 0 { section.Summary = "Loading..." } @@ -220,7 +220,7 @@ func BuildDossierSections(targetID, targetHex string, target *lib.Dossier, p *li default: // Generic handler for any category with a Category set if cfg.Category > 0 { - entries, _ := lib.EntryList("", "", cfg.Category, &lib.EntryFilter{DossierID: targetID, Limit: 50}) + entries, _ := lib.EntryList(lib.SystemAccessorID, "", cfg.Category, &lib.EntryFilter{DossierID: targetID, Limit: 50}) section.Items = entriesToSectionItems(entries) // Use section ID for summary (e.g., "2 medications" not "2 items") section.Summary = fmt.Sprintf("%d %s", len(entries), cfg.ID) @@ -366,7 +366,7 @@ func buildLabItems(dossierID, lang string, T func(string) string) ([]SectionItem orders, _ := lib.EntryQuery(dossierID, lib.CategoryLab, "lab_order") // Also get standalone lab results (no parent) - allLabs, _ := lib.EntryList("", "", lib.CategoryLab, &lib.EntryFilter{DossierID: dossierID, Limit: 5000}) + allLabs, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryLab, &lib.EntryFilter{DossierID: dossierID, Limit: 5000}) var standalones []*lib.Entry for _, e := range allLabs { if e.ParentID == "" && e.Type != "lab_order" { @@ -727,7 +727,7 @@ func handleDossierV2(w http.ResponseWriter, r *http.Request) { } // Check for genome - genomeEntries, _ := lib.EntryList("", "", lib.CategoryGenome, &lib.EntryFilter{DossierID: targetID, Limit: 1}) + genomeEntries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryGenome, &lib.EntryFilter{DossierID: targetID, Limit: 1}) hasGenome := len(genomeEntries) > 0 // Build sections diff --git a/portal/main.go b/portal/main.go index 0a3bbff..5878e67 100644 --- a/portal/main.go +++ b/portal/main.go @@ -913,7 +913,7 @@ func handleDemo(w http.ResponseWriter, r *http.Request) { for _, s := range studies { totalSlices += s.SliceCount } - genomeEntries, _ := lib.EntryList("", "", lib.CategoryGenome, &lib.EntryFilter{DossierID: demoDossierID, Limit: 1}) // nil ctx - demo lookup + genomeEntries, _ := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryGenome, &lib.EntryFilter{DossierID: demoDossierID, Limit: 1}) // nil ctx - demo lookup hasGenome := len(genomeEntries) > 0 // Build sections for demo dossier diff --git a/portal/upload.go b/portal/upload.go index b15cca7..faa0fc1 100644 --- a/portal/upload.go +++ b/portal/upload.go @@ -48,7 +48,7 @@ func formatBytes(b int64) string { func getUploads(dossierID string) []Upload { var uploads []Upload - entries, err := lib.EntryList("", "", lib.CategoryUpload, &lib.EntryFilter{ // nil ctx - internal operation + entries, err := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryUpload, &lib.EntryFilter{ // nil ctx - internal operation DossierID: dossierID, Limit: 50, }) @@ -104,7 +104,7 @@ func getUploadEntry(entryID, dossierID string) (filePath, fileName, category, st // findUploadByFilename finds existing uploads with the same filename func findUploadByFilename(dossierID, filename string) []*lib.Entry { - entries, err := lib.EntryList("", "", lib.CategoryUpload, &lib.EntryFilter{ // nil ctx - internal operation + entries, err := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryUpload, &lib.EntryFilter{ // nil ctx - internal operation DossierID: dossierID, Value: filename, }) @@ -399,7 +399,7 @@ func handleProcessImaging(w http.ResponseWriter, r *http.Request) { log("Access OK") // Get all uploads with status=uploaded (any type for now) - entries, err := lib.EntryList("", "", lib.CategoryUpload, &lib.EntryFilter{ + entries, err := lib.EntryList(lib.SystemAccessorID, "", lib.CategoryUpload, &lib.EntryFilter{ DossierID: targetID, }) if err != nil {