From c2331e7d5624ff00a4fa2773d20e6bcc5193e368 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Thu, 9 Jul 2026 08:49:07 -0300 Subject: [PATCH 1/2] fix(oscal-import): upsert existing documents and fix nested-dir recursion 'oscal import' used FirstOrCreate, which silently skips any document whose uuid already exists (while logging 'Successfully Created'). Content changes - new catalog controls, new implemented-requirements, edited statements - never reached the database on re-import, forcing API-side workarounds. Import now upserts: new uuids are created as before; existing documents are updated from the file with FullSaveAssociations, so nested rows (groups, controls, requirements, statements, by-components) are inserted or updated by primary key. Updates are additive - zero-value fields are skipped, so DB-managed columns absent from OSCAL (e.g. catalog Active) are preserved, and rows removed from a file are not deleted. Also fixes directory recursion: children were joined against the directory's base name instead of its opened path, so importing a directory containing a subdirectory panicked on the first nested file. Logs now say Created vs Updated, and log failures instead of unconditionally claiming success. Co-Authored-By: Claude Fable 5 --- cmd/oscal/import.go | 94 +++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/cmd/oscal/import.go b/cmd/oscal/import.go index 4f03a66f..ff9e2853 100644 --- a/cmd/oscal/import.go +++ b/cmd/oscal/import.go @@ -3,6 +3,7 @@ package oscal import ( "context" "encoding/json" + "errors" "io" "log" "os" @@ -75,6 +76,51 @@ func importOscal(cmd *cobra.Command, args []string) { } } +// importResult reports what upsertDocument did with a document. +type importResult struct { + created bool + err error +} + +// upsertDocument creates the document when its uuid is new, and otherwise +// updates it from the file contents (FullSaveAssociations upserts every +// nested row by primary key). Re-importing an updated file therefore +// propagates new controls, requirements and statements instead of being +// silently skipped. +// +// Update semantics are additive: struct-based Updates skips zero-value +// fields, so DB-managed columns not represented in OSCAL (e.g. a catalog's +// Active flag) are left untouched, and rows or values removed from the file +// are NOT deleted or cleared. +func upsertDocument[T any](db *gorm.DB, rawID string, def *T) importResult { + id, err := uuid.Parse(rawID) + if err != nil { + return importResult{err: err} + } + + var existing T + err = db.Select("id").First(&existing, "id = ?", id).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return importResult{created: true, err: db.Create(def).Error} + } + if err != nil { + return importResult{err: err} + } + return importResult{err: db.Session(&gorm.Session{FullSaveAssociations: true}).Updates(def).Error} +} + +func logImport(sugar *zap.SugaredLogger, res importResult, kind, title, file string) { + if res.err != nil { + sugar.Errorw("Failed to import "+kind, "title", title, "file", file, "error", res.err) + return + } + action := "Updated" + if res.created { + action = "Created" + } + sugar.Infow("Successfully "+action+" "+kind, "title", title, "file", file) +} + func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { info, err := f.Stat() if err != nil { @@ -93,7 +139,9 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { continue } - systemFile, err := os.Open(path.Join(info.Name(), dirFile.Name())) + // Join against the path the directory was opened with (f.Name()), + // not its base name (info.Name()), so nested directories resolve. + systemFile, err := os.Open(path.Join(f.Name(), dirFile.Name())) if err != nil { panic(err) } @@ -135,65 +183,42 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { if input.Catalog != nil { def := &relational.Catalog{} def.UnmarshalOscal(*input.Catalog) - out := db.FirstOrCreate(def) - if out.Error != nil { - sugar.Error(out.Error) - } - sugar.Infow("Successfully Created Catalog", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.Catalog.UUID, def), "Catalog", def.Metadata.Title, f.Name()) imported = true } if input.ComponentDefinition != nil { def := &relational.ComponentDefinition{} def.UnmarshalOscal(*input.ComponentDefinition) - out := db.FirstOrCreate(def) - if out.Error != nil { - sugar.Error(out.Error) - } - sugar.Infow("Successfully Created Component Definition", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.ComponentDefinition.UUID, def), "Component Definition", def.Metadata.Title, f.Name()) imported = true } if input.SystemSecurityPlan != nil { def := &relational.SystemSecurityPlan{} def.UnmarshalOscal(*input.SystemSecurityPlan) - out := db.FirstOrCreate(def) - if out.Error != nil { - sugar.Error(out.Error) - } - sugar.Infow("Successfully Created System Security Plan", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.SystemSecurityPlan.UUID, def), "System Security Plan", def.Metadata.Title, f.Name()) imported = true } if input.AssessmentPlan != nil { def := &relational.AssessmentPlan{} def.UnmarshalOscal(*input.AssessmentPlan) - out := db.FirstOrCreate(def) - if out.Error != nil { - panic(out.Error) - } - sugar.Infow("Successfully Created Assessment Plan", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.AssessmentPlan.UUID, def), "Assessment Plan", def.Metadata.Title, f.Name()) imported = true } if input.AssessmentResult != nil { def := &relational.AssessmentResult{} def.UnmarshalOscal(*input.AssessmentResult) - out := db.FirstOrCreate(def) - if out.Error != nil { - panic(out.Error) - } - sugar.Infow("Successfully Created Assessment Result", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.AssessmentResult.UUID, def), "Assessment Result", def.Metadata.Title, f.Name()) imported = true } if input.Profile != nil { def := &relational.Profile{} def.UnmarshalOscal(*input.Profile) - out := db.FirstOrCreate(def) - if out.Error != nil { - panic(out.Error) - } + logImport(sugar, upsertDocument(db, input.Profile.UUID, def), "Profile", def.Metadata.Title, f.Name()) // Sync ProfileControl pivot table synchronously so errors can be reported _, err := oscal.SyncProfileControls(db, uuid.MustParse(input.Profile.UUID)) @@ -202,7 +227,6 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { return err } - sugar.Infow("Successfully Created Profile", "title", def.Metadata.Title, "file", f.Name()) imported = true } @@ -214,13 +238,7 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { sugar.Infof("Importing POAM with %d risks, %d observations, %d findings", len(def.Risks), len(def.Observations), len(def.Findings)) - // Create with polymorphic entities - out := db.FirstOrCreate(def) - if out.Error != nil { - sugar.Errorf("Error creating POAM: %v", out.Error) - return err - } - sugar.Infow("Successfully Created Plan of Action and Milestones", "title", def.Metadata.Title, "file", f.Name()) + logImport(sugar, upsertDocument(db, input.PlanOfActionAndMilestones.UUID, def), "Plan of Action and Milestones", def.Metadata.Title, f.Name()) imported = true } From f820574be886a7ec79a3b32e8ac080824fcfd289 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Thu, 9 Jul 2026 09:07:45 -0300 Subject: [PATCH 2/2] fix(oscal-import): dedupe singleton associations, propagate errors to exit status Review follow-ups: - Updates+FullSaveAssociations does upsert nested rows (verified: edited by-component descriptions/statuses and new requirements land on re-import), but association rows without a stable OSCAL identifier - metadata and back-matter, whose ids are DB-generated - were duplicated on every re-import. reuseSingletonAssociations now points them at the existing rows so updates modify in place. - Failures now reach the exit status. logImport returns the error, importFile joins errors across documents and directory entries (continuing past bad files instead of aborting the batch), and importOscal exits non-zero when any document failed. A panic while unmarshalling a malformed file (the relational layer uses uuid.MustParse) is recovered per file and converted into an error. Co-Authored-By: Claude Fable 5 --- cmd/oscal/import.go | 137 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 106 insertions(+), 31 deletions(-) diff --git a/cmd/oscal/import.go b/cmd/oscal/import.go index ff9e2853..2ba14b5d 100644 --- a/cmd/oscal/import.go +++ b/cmd/oscal/import.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "log" "os" @@ -62,17 +63,21 @@ func importOscal(cmd *cobra.Command, args []string) { panic("failed to connect database") } + var errs error for _, f := range files { systemFile, err := os.Open(f) if err != nil { - panic(err) + errs = errors.Join(errs, err) + sugar.Errorw("Failed to open import path", "path", f, "error", err) + continue } - err = importFile(db, sugar, systemFile) - if err != nil { - panic(err) + if err := importFile(db, sugar, systemFile); err != nil { + errs = errors.Join(errs, err) } - + } + if errs != nil { + sugar.Fatalw("Import finished with errors", "error", errs) } } @@ -106,19 +111,74 @@ func upsertDocument[T any](db *gorm.DB, rawID string, def *T) importResult { if err != nil { return importResult{err: err} } + reuseSingletonAssociations(db, id, def) return importResult{err: db.Session(&gorm.Session{FullSaveAssociations: true}).Updates(def).Error} } -func logImport(sugar *zap.SugaredLogger, res importResult, kind, title, file string) { +// reuseSingletonAssociations points has-one association rows that carry no +// stable OSCAL identifier of their own (metadata, back-matter — their ids are +// DB-generated) at the existing rows for this document, so an update modifies +// them in place. Without this, every re-import would insert a duplicate +// metadata/back-matter row alongside the old one. +func reuseSingletonAssociations(db *gorm.DB, parentID uuid.UUID, def any) { + reuseMetadata := func(parentType string, meta *relational.Metadata) { + var existing relational.Metadata + if err := db.Select("id"). + First(&existing, "parent_id = ? AND parent_type = ?", parentID.String(), parentType). + Error; err == nil { + meta.ID = existing.ID + } + } + reuseBackMatter := func(parentType string, bm *relational.BackMatter) { + if bm == nil { + return + } + var existing relational.BackMatter + if err := db.Select("id"). + First(&existing, "parent_id = ? AND parent_type = ?", parentID.String(), parentType). + Error; err == nil { + bm.ID = existing.ID + } + } + + switch d := def.(type) { + case *relational.Catalog: + reuseMetadata("catalogs", &d.Metadata) + reuseBackMatter("catalogs", d.BackMatter) + case *relational.ComponentDefinition: + reuseMetadata("component_definitions", &d.Metadata) + reuseBackMatter("component_definitions", &d.BackMatter) + case *relational.SystemSecurityPlan: + reuseMetadata("system_security_plans", &d.Metadata) + reuseBackMatter("system_security_plans", d.BackMatter) + case *relational.AssessmentPlan: + reuseMetadata("assessment_plans", &d.Metadata) + reuseBackMatter("assessment_plans", d.BackMatter) + case *relational.AssessmentResult: + reuseMetadata("assessment_results", &d.Metadata) + reuseBackMatter("assessment_results", d.BackMatter) + case *relational.Profile: + reuseMetadata("profiles", &d.Metadata) + reuseBackMatter("profiles", d.BackMatter) + case *relational.PlanOfActionAndMilestones: + reuseMetadata("plan_of_action_and_milestones", &d.Metadata) + reuseBackMatter("plan_of_action_and_milestones", &d.BackMatter) + } +} + +// logImport reports the result and returns the error (if any) so callers can +// propagate it into the process exit status. +func logImport(sugar *zap.SugaredLogger, res importResult, kind, title, file string) error { if res.err != nil { sugar.Errorw("Failed to import "+kind, "title", title, "file", file, "error", res.err) - return + return res.err } action := "Updated" if res.created { action = "Created" } sugar.Infow("Successfully "+action+" "+kind, "title", title, "file", file) + return nil } func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { @@ -134,6 +194,10 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { return err } + // Import every entry even when one fails, and report the failures + // together so a bad file surfaces in the exit status without + // aborting the rest of the directory. + var errs error for _, dirFile := range files { if dirFile.Name()[0:1] == "." { continue @@ -143,7 +207,8 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { // not its base name (info.Name()), so nested directories resolve. systemFile, err := os.Open(path.Join(f.Name(), dirFile.Name())) if err != nil { - panic(err) + errs = errors.Join(errs, err) + continue } defer func() { err := systemFile.Close() @@ -152,16 +217,31 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { } }() - err = importFile(db, sugar, systemFile) - if err != nil { - panic(err) + if err := importFile(db, sugar, systemFile); err != nil { + errs = errors.Join(errs, err) } } - return nil + return errs } - sugar.Infow("Importing file", "file", info.Name()) + return importDocuments(db, sugar, f) +} + +// importDocuments imports the OSCAL documents in a single file. A panic while +// unmarshalling or persisting (e.g. a malformed uuid in the file — the +// relational layer uses uuid.MustParse) is converted into an error so one bad +// file fails the import's exit status without aborting the rest of the batch. +func importDocuments(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) (errs error) { + defer func() { + if r := recover(); r != nil { + err := fmt.Errorf("importing %s panicked: %v", f.Name(), r) + sugar.Errorw("Failed to import file", "file", f.Name(), "error", err) + errs = errors.Join(errs, err) + } + }() + + sugar.Infow("Importing file", "file", f.Name()) input := &struct { ComponentDefinition *oscalTypes_1_1_3.ComponentDefinition `json:"component-definition"` @@ -173,8 +253,7 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { PlanOfActionAndMilestones *oscalTypes_1_1_3.PlanOfActionAndMilestones `json:"plan-of-action-and-milestones"` }{} - err = json.NewDecoder(f).Decode(input) - if err != nil { + if err := json.NewDecoder(f).Decode(input); err != nil { sugar.Error(err) } @@ -183,48 +262,47 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { if input.Catalog != nil { def := &relational.Catalog{} def.UnmarshalOscal(*input.Catalog) - logImport(sugar, upsertDocument(db, input.Catalog.UUID, def), "Catalog", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.Catalog.UUID, def), "Catalog", def.Metadata.Title, f.Name())) imported = true } if input.ComponentDefinition != nil { def := &relational.ComponentDefinition{} def.UnmarshalOscal(*input.ComponentDefinition) - logImport(sugar, upsertDocument(db, input.ComponentDefinition.UUID, def), "Component Definition", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.ComponentDefinition.UUID, def), "Component Definition", def.Metadata.Title, f.Name())) imported = true } if input.SystemSecurityPlan != nil { def := &relational.SystemSecurityPlan{} def.UnmarshalOscal(*input.SystemSecurityPlan) - logImport(sugar, upsertDocument(db, input.SystemSecurityPlan.UUID, def), "System Security Plan", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.SystemSecurityPlan.UUID, def), "System Security Plan", def.Metadata.Title, f.Name())) imported = true } if input.AssessmentPlan != nil { def := &relational.AssessmentPlan{} def.UnmarshalOscal(*input.AssessmentPlan) - logImport(sugar, upsertDocument(db, input.AssessmentPlan.UUID, def), "Assessment Plan", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.AssessmentPlan.UUID, def), "Assessment Plan", def.Metadata.Title, f.Name())) imported = true } if input.AssessmentResult != nil { def := &relational.AssessmentResult{} def.UnmarshalOscal(*input.AssessmentResult) - logImport(sugar, upsertDocument(db, input.AssessmentResult.UUID, def), "Assessment Result", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.AssessmentResult.UUID, def), "Assessment Result", def.Metadata.Title, f.Name())) imported = true } if input.Profile != nil { def := &relational.Profile{} def.UnmarshalOscal(*input.Profile) - logImport(sugar, upsertDocument(db, input.Profile.UUID, def), "Profile", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.Profile.UUID, def), "Profile", def.Metadata.Title, f.Name())) // Sync ProfileControl pivot table synchronously so errors can be reported - _, err := oscal.SyncProfileControls(db, uuid.MustParse(input.Profile.UUID)) - if err != nil { + if _, err := oscal.SyncProfileControls(db, uuid.MustParse(input.Profile.UUID)); err != nil { sugar.Errorw("Failed to sync profile controls", "error", err) - return err + errs = errors.Join(errs, err) } imported = true @@ -238,24 +316,21 @@ func importFile(db *gorm.DB, sugar *zap.SugaredLogger, f *os.File) error { sugar.Infof("Importing POAM with %d risks, %d observations, %d findings", len(def.Risks), len(def.Observations), len(def.Findings)) - logImport(sugar, upsertDocument(db, input.PlanOfActionAndMilestones.UUID, def), "Plan of Action and Milestones", def.Metadata.Title, f.Name()) + errs = errors.Join(errs, logImport(sugar, upsertDocument(db, input.PlanOfActionAndMilestones.UUID, def), "Plan of Action and Milestones", def.Metadata.Title, f.Name())) imported = true } if imported { - return nil + return errs } // Reset the file to the beginning. We'll read it again. - _, err = f.Seek(0, io.SeekStart) - if err != nil { + if _, err := f.Seek(0, io.SeekStart); err != nil { return err } output := &map[string]any{} - decoder := json.NewDecoder(f) - err = decoder.Decode(output) - if err != nil { + if err := json.NewDecoder(f).Decode(output); err != nil { sugar.Error(err) return err }