From 3dc4e840ca9b8b445674d2b0535153e601245a24 Mon Sep 17 00:00:00 2001 From: Amen <2668333+amenophis1er@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:28:35 -0400 Subject: [PATCH] fix(restore): restore documents before uploading media, fail fast on a read-only token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A token with content:read/media:read but no content:write made 'trokky restore' upload every media file first — minting fresh media IDs — then fail every document write with HTTP 403, then print "Restore completed" and exit 0. The target instance was left with a full media store under new IDs while its documents still referenced the old ones, so every image 404s. Restore is now three ordered phases: documents are written first with their references untouched, media is uploaded second, and a third pass rewrites media references only into documents that carry them. Media upload is the destructive half, so nothing is uploaded until the token has proven it can write documents; the first 401/403 on a document write aborts with "token lacks content:write (required to restore documents)" and a non-zero exit, before any upload happens. The PUT->POST singleton fallback and the --overwrite POST->PUT fallback no longer mask a permission denial. Summary counters now report work that actually landed: documents written, media uploaded, references contained in documents that were successfully rewritten, and collections that actually received documents. "Restore completed" prints only on a fully clean run; any failure exits non-zero. If media was uploaded and some documents could not be pointed at it, a loud orphaned-media warning names the counts. Also normalize --url and TROKKY_URL to the API root, so '--url http://host:3210' no longer fails with "HTTP 404: Cannot GET /collections". Closes #2 --- cmd/documents_test.go | 6 +- cmd/restore.go | 513 +++++++++++++++++++++++---------- cmd/restore_test.go | 467 ++++++++++++++++++++++++++++++ internal/config/config.go | 42 ++- internal/config/config_test.go | 111 ++++++- 5 files changed, 971 insertions(+), 168 deletions(-) create mode 100644 cmd/restore_test.go diff --git a/cmd/documents_test.go b/cmd/documents_test.go index 0fa0089..2acb58d 100644 --- a/cmd/documents_test.go +++ b/cmd/documents_test.go @@ -174,8 +174,10 @@ func TestDocumentsListSendsV2QueryParams(t *testing.T) { t.Fatalf("command failed: %v", err) } - if gotPath != "/collections/posts" { - t.Errorf("path = %q, want %q", gotPath, "/collections/posts") + // A --url without an /api suffix is normalized to the API root, so the + // request lands on /api/collections/posts. + if gotPath != "/api/collections/posts" { + t.Errorf("path = %q, want %q", gotPath, "/api/collections/posts") } if got := gotQuery.Get("sort"); got != "-_createdAt" { t.Errorf("sort = %q, want %q", got, "-_createdAt") diff --git a/cmd/restore.go b/cmd/restore.go index 9c82c5c..fc891da 100644 --- a/cmd/restore.go +++ b/cmd/restore.go @@ -14,16 +14,35 @@ import ( "github.com/trokky/cli/internal/client" ) +// restoredDoc records a document written during phase 1 so that phase 3 can +// rewrite its media references once the new media IDs are known. +type restoredDoc struct { + collection string + id string + body map[string]interface{} + schema backup.SchemaDefinition + hasSchema bool +} + var restoreCmd = &cobra.Command{ Use: "restore", Short: "Restore content from a Trokky backup file", Long: `Restore content and media from a zip backup created by 'trokky backup'. +Documents are restored first, media is uploaded second, and references are +rewritten last. If the token cannot write documents the restore aborts before +anything is uploaded, so a half-restored instance is never left behind. + Example: trokky restore --input backup.zip trokky restore --input backup.zip --collections posts,pages trokky restore --input backup.zip --dry-run`, + // A failed restore must not bury its error under a usage dump. + SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + out := cmd.OutOrStdout() + errOut := cmd.ErrOrStderr() + c, err := client.FromContext(cmd) if err != nil { return err @@ -58,9 +77,9 @@ Example: manifest.Schemas[i].UnmarshalFields() } - fmt.Printf("Backup from %s\n", manifest.Timestamp) + fmt.Fprintf(out, "Backup from %s\n", manifest.Timestamp) if manifest.Source.URL != "" { - fmt.Printf("Source: %s\n", manifest.Source.URL) + fmt.Fprintf(out, "Source: %s\n", manifest.Source.URL) } // Determine collections to restore @@ -102,7 +121,7 @@ Example: } collectionsToRestore = requested - fmt.Printf("Selected %d collection(s) for restore\n", len(collectionsToRestore)) + fmt.Fprintf(out, "Selected %d collection(s) for restore\n", len(collectionsToRestore)) } // Build schema map and restore order @@ -131,23 +150,23 @@ Example: } } - fmt.Printf("Restore order: %v\n", restoreOrder) + fmt.Fprintf(out, "Restore order: %v\n", restoreOrder) if dryRun { - fmt.Println("\n[DRY RUN] No changes will be made") + fmt.Fprintln(out, "\n[DRY RUN] No changes will be made") } // Pre-flight schema validation - fmt.Print("Validating target instance... ") + fmt.Fprint(out, "Validating target instance... ") targetData, err := c.Get("/collections") if err != nil { - fmt.Println("failed") + fmt.Fprintln(out, "failed") return fmt.Errorf("failed to fetch target schemas: %w", err) } targetSchemas, err := backup.ParseSchemas(targetData) if err != nil { - fmt.Println("failed") + fmt.Fprintln(out, "failed") return fmt.Errorf("failed to parse target schemas: %w", err) } @@ -166,18 +185,18 @@ Example: validation := backup.ValidateSchemaCompatibility(schemasToValidate, targetSchemas) if !validation.Compatible { - fmt.Println("failed") + fmt.Fprintln(out, "failed") for _, e := range validation.Errors { - fmt.Printf(" - %s\n", e) + fmt.Fprintf(out, " - %s\n", e) } return fmt.Errorf("schema validation failed") } - fmt.Println("passed") + fmt.Fprintln(out, "passed") // Clean existing data if requested if clean && !dryRun { // Clean documents - fmt.Print("Cleaning existing documents... ") + fmt.Fprint(out, "Cleaning existing documents... ") deletedDocs := 0 for _, collection := range collectionsToRestore { data, err := c.Get("/collections/" + collection + "?limit=10000") @@ -194,10 +213,10 @@ Example: } } } - fmt.Printf("%d document(s) deleted\n", deletedDocs) + fmt.Fprintf(out, "%d document(s) deleted\n", deletedDocs) // Clean media (paginated with rate-limit handling) - fmt.Print("Cleaning existing media... ") + fmt.Fprint(out, "Cleaning existing media... ") deletedMedia := 0 for { mediaData, err := c.Get("/media?limit=100") @@ -223,99 +242,34 @@ Example: } time.Sleep(100 * time.Millisecond) } - fmt.Printf("\r Cleaning existing media... %d deleted", deletedMedia) + fmt.Fprintf(out, "\r Cleaning existing media... %d deleted", deletedMedia) } - fmt.Printf("\r Cleaning existing media... %d file(s) deleted\n", deletedMedia) + fmt.Fprintf(out, "\r Cleaning existing media... %d file(s) deleted\n", deletedMedia) } - // ID mappings for reference rewriting - idMappings := make(map[string]string) - mediaRestored := 0 - - // Restore media first (builds old ID -> new ID mappings) - if len(manifest.MediaIndex) > 0 { - if dryRun { - fmt.Printf(" [DRY RUN] Would restore %d media file(s)\n", len(manifest.MediaIndex)) - } else { - fmt.Printf(" Restoring media... ") - - // Build zip file lookup for media - mediaZipFiles := make(map[string]*zip.File) - for _, f := range zr.File { - if strings.HasPrefix(f.Name, "media/") { - // Strip "media/" prefix to get filename - name := f.Name[6:] - mediaZipFiles[name] = f - } - } - - mediaTotal := len(manifest.MediaIndex) - mediaFailed := 0 - i := 0 - - for oldID, mediaInfo := range manifest.MediaIndex { - i++ - zipFile, ok := mediaZipFiles[mediaInfo.Filename] - if !ok { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: media file %s not found in archive\n", mediaInfo.Filename) - mediaFailed++ - continue - } - - // Retry up to 3 times with backoff - var result map[string]interface{} - var uploadErr error - for attempt := 1; attempt <= 3; attempt++ { - rc, err := zipFile.Open() - if err != nil { - uploadErr = err - break - } - result, uploadErr = c.UploadFile(mediaInfo.Filename, rc) - rc.Close() - if uploadErr == nil { - break - } - if attempt < 3 { - delay := time.Duration(attempt) * 2 * time.Second - time.Sleep(delay) - } - } - - if uploadErr != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: failed to upload %s (after 3 attempts): %v\n", mediaInfo.Filename, uploadErr) - mediaFailed++ - continue - } - - // Extract new ID from upload response - newID := extractMediaID(result) - if newID != "" { - idMappings[oldID] = newID - mediaRestored++ - } else { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: uploaded %s but could not extract new ID\n", mediaInfo.Filename) - mediaFailed++ - } - - // Progress indicator - fmt.Fprintf(cmd.OutOrStdout(), "\r Restoring media... %d/%d (%d failed)", mediaRestored, mediaTotal, mediaFailed) - - // Small delay between uploads to avoid overwhelming the server - time.Sleep(200 * time.Millisecond) - } - - fmt.Fprintf(cmd.OutOrStdout(), "\r Restoring media... %d/%d file(s) \n", mediaRestored, mediaTotal) - if mediaFailed > 0 { - fmt.Fprintf(cmd.ErrOrStderr(), " %d file(s) failed\n", mediaFailed) - } - } + if dryRun && len(manifest.MediaIndex) > 0 { + fmt.Fprintf(out, " [DRY RUN] Would restore %d media file(s)\n", len(manifest.MediaIndex)) } - // Restore documents in order + // Document ID mappings (old ID -> new ID), used to rewrite document-to-document + // references as later collections are restored. Media mappings are deliberately + // kept separate: they only exist after phase 2. + idMappings := make(map[string]string) + mediaIDMappings := make(map[string]string) + + totalDocuments := 0 totalRestored := 0 + totalFailed := 0 totalRefsUpdated := 0 + collectionsRestored := 0 + mediaRestored := 0 + staleRefDocs := 0 + var restoredDocs []restoredDoc + // ── Phase 1: restore documents ──────────────────────────────────────── + // Documents go first because media upload is the destructive half: every + // upload mints a brand-new media ID. If the token cannot write documents + // we must find out before a single byte is uploaded. for _, collectionName := range restoreOrder { schema, hasSchema := schemaMap[collectionName] @@ -333,34 +287,39 @@ Example: } if len(docFiles) == 0 { - fmt.Printf(" %s: no documents\n", collectionName) + fmt.Fprintf(out, " %s: no documents\n", collectionName) continue } if dryRun { - fmt.Printf(" [DRY RUN] Would restore %d document(s) to %s\n", len(docFiles), collectionName) + fmt.Fprintf(out, " [DRY RUN] Would restore %d document(s) to %s\n", len(docFiles), collectionName) continue } - fmt.Printf(" Restoring %s... ", collectionName) + totalDocuments += len(docFiles) + fmt.Fprintf(out, " Restoring %s... ", collectionName) restored := 0 + failed := 0 for _, f := range docFiles { rc, err := f.Open() if err != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: failed to open %s: %v\n", f.Name, err) + fmt.Fprintf(errOut, "\n Warning: failed to open %s: %v\n", f.Name, err) + failed++ continue } data, err := io.ReadAll(rc) rc.Close() if err != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: failed to read %s: %v\n", f.Name, err) + fmt.Fprintf(errOut, "\n Warning: failed to read %s: %v\n", f.Name, err) + failed++ continue } var doc map[string]interface{} if err := json.Unmarshal(data, &doc); err != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: invalid JSON in %s: %v\n", f.Name, err) + fmt.Fprintf(errOut, "\n Warning: invalid JSON in %s: %v\n", f.Name, err) + failed++ continue } @@ -369,17 +328,18 @@ Example: // Strip system fields cleanDoc := backup.StripSystemFields(doc) - // Update references (schema-aware + deep scan + string replacement) + // Rewrite document-to-document references using the mappings built + // so far. Media references are left exactly as the backup has them; + // phase 3 fixes those once the new media IDs exist. + docRefs := 0 if len(idMappings) > 0 { if hasSchema && len(schema.Fields) > 0 { var refCount int cleanDoc, refCount = backup.UpdateReferences(cleanDoc, schema, idMappings) - totalRefsUpdated += refCount + docRefs += refCount } - // Deep scan as safety net — catches any asset._ref the schema pass missed - totalRefsUpdated += backup.DeepUpdateMediaRefs(cleanDoc, idMappings) - // String replacement — rewrites media IDs embedded in richtext HTML - totalRefsUpdated += backup.DeepReplaceMediaIDsInStrings(cleanDoc, idMappings) + docRefs += backup.DeepUpdateMediaRefs(cleanDoc, idMappings) + docRefs += backup.DeepReplaceMediaIDsInStrings(cleanDoc, idMappings) } // Sanitize @@ -388,89 +348,330 @@ Example: // Create/update document docJSON, err := json.Marshal(map[string]interface{}{"data": cleanDoc}) if err != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: failed to marshal doc: %v\n", err) + fmt.Fprintf(errOut, "\n Warning: failed to marshal doc: %v\n", err) + failed++ continue } + body := func() io.Reader { return bytes.NewReader(docJSON) } var respData []byte + var writeErr error + var firstErr error + if isSingleton && originalID != "" { // Singleton: use PUT to upsert with original ID - respData, err = c.Put("/collections/"+collectionName+"/"+originalID, bytes.NewReader(docJSON)) - if err != nil { + respData, writeErr = c.Put("/collections/"+collectionName+"/"+originalID, body()) + if writeErr != nil { // PUT failed — log and fallback to POST - fmt.Fprintf(cmd.ErrOrStderr(), "\n Note: PUT failed for singleton %s/%s (%v), trying POST\n", collectionName, originalID, err) - respData, err = c.Post("/collections/"+collectionName, bytes.NewReader(docJSON)) + firstErr = writeErr + fmt.Fprintf(errOut, "\n Note: PUT failed for singleton %s/%s (%v), trying POST\n", collectionName, originalID, writeErr) + respData, writeErr = c.Post("/collections/"+collectionName, body()) } } else { // Regular document: POST to create - respData, err = c.Post("/collections/"+collectionName, bytes.NewReader(docJSON)) + respData, writeErr = c.Post("/collections/"+collectionName, body()) + if writeErr != nil && overwrite && originalID != "" { + firstErr = writeErr + respData, writeErr = c.Put("/collections/"+collectionName+"/"+originalID, body()) + } } - if err != nil { - if overwrite && originalID != "" { - if _, putErr := c.Put("/collections/"+collectionName+"/"+originalID, bytes.NewReader(docJSON)); putErr != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "\n Warning: failed to create/update doc %s: %v\n", originalID, putErr) - } else { - idMappings[originalID] = originalID - restored++ + if writeErr != nil { + // A permission denial must never be masked by the PUT/POST + // fallbacks: abort now, before anything is uploaded, so the + // target instance cannot be left half-restored. + if isPermissionDenied(writeErr) || isPermissionDenied(firstErr) { + cause := firstErr + if cause == nil { + cause = writeErr } + fmt.Fprintln(out) + return fmt.Errorf("token lacks content:write (required to restore documents): %w", cause) } + fmt.Fprintf(errOut, "\n Warning: failed to create/update doc %s: %v\n", originalID, writeErr) + failed++ continue } - // Extract new ID for mappings - if originalID != "" { - if isSingleton { - // Singletons preserve their ID + // Extract the target ID so phase 3 knows where to PUT the fixed document. + newID := originalID + if isSingleton { + if originalID != "" { idMappings[originalID] = originalID - } else { - var result map[string]interface{} - if json.Unmarshal(respData, &result) == nil { - newID := backup.ExtractDocID(result) - if newID == "" { - if docResult, ok := result["document"].(map[string]interface{}); ok { - newID = backup.ExtractDocID(docResult) - } - } - if newID != "" { - idMappings[originalID] = newID + } + } else { + var result map[string]interface{} + if len(respData) > 0 && json.Unmarshal(respData, &result) == nil { + parsed := backup.ExtractDocID(result) + if parsed == "" { + if docResult, ok := result["document"].(map[string]interface{}); ok { + parsed = backup.ExtractDocID(docResult) } } + if parsed != "" { + newID = parsed + } + } + if originalID != "" && newID != "" { + idMappings[originalID] = newID } } restored++ + totalRefsUpdated += docRefs + restoredDocs = append(restoredDocs, restoredDoc{ + collection: collectionName, + id: newID, + body: cleanDoc, + schema: schema, + hasSchema: hasSchema, + }) } totalRestored += restored - fmt.Printf("%d/%d document(s)\n", restored, len(docFiles)) + totalFailed += failed + if restored > 0 { + collectionsRestored++ + } + fmt.Fprintf(out, "%d/%d document(s)\n", restored, len(docFiles)) + if failed > 0 { + fmt.Fprintf(errOut, " %d document(s) failed\n", failed) + } + } + + // ── Phase 2: upload media ───────────────────────────────────────────── + // Only ever reached when phase 1 actually wrote something, so uploads + // cannot orphan themselves against a document set that was never written. + if !dryRun && len(manifest.MediaIndex) > 0 { + if totalRestored == 0 { + fmt.Fprintf(errOut, " Skipping media upload: no documents were restored\n") + } else { + fmt.Fprint(out, " Restoring media... ") + + // Build zip file lookup for media + mediaZipFiles := make(map[string]*zip.File) + for _, f := range zr.File { + if strings.HasPrefix(f.Name, "media/") { + // Strip "media/" prefix to get filename + name := f.Name[6:] + mediaZipFiles[name] = f + } + } + + mediaTotal := len(manifest.MediaIndex) + mediaFailed := 0 + + for oldID, mediaInfo := range manifest.MediaIndex { + zipFile, ok := mediaZipFiles[mediaInfo.Filename] + if !ok { + fmt.Fprintf(errOut, "\n Warning: media file %s not found in archive\n", mediaInfo.Filename) + mediaFailed++ + continue + } + + // Retry up to 3 times with backoff + var result map[string]interface{} + var uploadErr error + for attempt := 1; attempt <= 3; attempt++ { + rc, err := zipFile.Open() + if err != nil { + uploadErr = err + break + } + result, uploadErr = c.UploadFile(mediaInfo.Filename, rc) + rc.Close() + if uploadErr == nil { + break + } + if attempt < 3 { + delay := time.Duration(attempt) * 2 * time.Second + time.Sleep(delay) + } + } + + if uploadErr != nil { + fmt.Fprintf(errOut, "\n Warning: failed to upload %s (after 3 attempts): %v\n", mediaInfo.Filename, uploadErr) + mediaFailed++ + continue + } + + // Extract new ID from upload response + newID := extractMediaID(result) + if newID != "" { + mediaIDMappings[oldID] = newID + mediaRestored++ + } else { + fmt.Fprintf(errOut, "\n Warning: uploaded %s but could not extract new ID\n", mediaInfo.Filename) + mediaFailed++ + } + + // Progress indicator + fmt.Fprintf(out, "\r Restoring media... %d/%d (%d failed)", mediaRestored, mediaTotal, mediaFailed) + + // Small delay between uploads to avoid overwhelming the server + time.Sleep(200 * time.Millisecond) + } + + fmt.Fprintf(out, "\r Restoring media... %d/%d file(s) \n", mediaRestored, mediaTotal) + if mediaFailed > 0 { + fmt.Fprintf(errOut, " %d file(s) failed\n", mediaFailed) + } + } + } + + // ── Phase 3: rewrite media references ───────────────────────────────── + // Documents without media references are never rewritten. + if len(mediaIDMappings) > 0 && len(restoredDocs) > 0 { + fmt.Fprint(out, " Updating media references... ") + updatedDocs := 0 + + for _, rd := range restoredDocs { + fixed, refCount, err := applyMediaReferences(rd, mediaIDMappings) + if err != nil { + fmt.Fprintf(errOut, "\n Warning: failed to rewrite references in %s: %v\n", rd.collection, err) + staleRefDocs++ + continue + } + if refCount == 0 { + continue + } + if rd.id == "" { + fmt.Fprintf(errOut, "\n Warning: restored document in %s has no known ID, cannot update its media references\n", rd.collection) + staleRefDocs++ + continue + } + + payload, err := json.Marshal(map[string]interface{}{"data": fixed}) + if err != nil { + fmt.Fprintf(errOut, "\n Warning: failed to marshal %s/%s: %v\n", rd.collection, rd.id, err) + staleRefDocs++ + continue + } + + if _, err := c.Put("/collections/"+rd.collection+"/"+rd.id, bytes.NewReader(payload)); err != nil { + fmt.Fprintf(errOut, "\n Warning: failed to update media references in %s/%s: %v\n", rd.collection, rd.id, err) + staleRefDocs++ + continue + } + + totalRefsUpdated += refCount + updatedDocs++ + } + + fmt.Fprintf(out, "%d document(s) updated\n", updatedDocs) + + } + + // Any document that was not written, or was written but could not be + // pointed at the new media IDs, leaves uploaded media orphaned. + if mediaRestored > 0 && staleRefDocs+totalFailed > 0 { + fmt.Fprintf(errOut, "\n!! WARNING: ORPHANED MEDIA !!\n") + fmt.Fprintf(errOut, "%d media file(s) were uploaded under new IDs, but %d document(s) could not be updated to reference them.\n", mediaRestored, staleRefDocs+totalFailed) + fmt.Fprintf(errOut, "That media is orphaned and those documents' images will 404.\n") + fmt.Fprintf(errOut, "No rollback was attempted: deleting the media would break the documents that were updated.\n") + fmt.Fprintf(errOut, "Re-run the restore with a token that can write documents, or repair the affected documents by hand.\n") } // Summary - fmt.Println() - if dryRun { - fmt.Println("Dry run completed - no changes made") - } else { - fmt.Println("Restore completed") - } - fmt.Println() - fmt.Println("Restore Summary") - fmt.Println("──────────────────────────────────────────────────") - fmt.Printf("Documents restored: %d\n", totalRestored) - fmt.Printf("Media restored: %d\n", mediaRestored) - fmt.Printf("References updated: %d\n", totalRefsUpdated) - fmt.Printf("Collections: %d\n", len(collectionsToRestore)) + fmt.Fprintln(out) + switch { + case dryRun: + fmt.Fprintln(out, "Dry run completed - no changes made") + case totalRestored == 0 && totalDocuments > 0: + fmt.Fprintln(out, "Restore FAILED - no documents were restored") + case totalFailed > 0: + fmt.Fprintf(out, "Restore incomplete - %d document(s) restored, %d failed\n", totalRestored, totalFailed) + case staleRefDocs > 0: + fmt.Fprintf(out, "Restore incomplete - %d document(s) still reference the old media IDs\n", staleRefDocs) + default: + fmt.Fprintln(out, "Restore completed") + } + fmt.Fprintln(out) + fmt.Fprintln(out, "Restore Summary") + fmt.Fprintln(out, "──────────────────────────────────────────────────") + fmt.Fprintf(out, "Documents restored: %d\n", totalRestored) + if totalFailed > 0 { + fmt.Fprintf(out, "Documents failed: %d\n", totalFailed) + } + fmt.Fprintf(out, "Media restored: %d\n", mediaRestored) + fmt.Fprintf(out, "References updated: %d\n", totalRefsUpdated) if dryRun { - fmt.Printf("Mode: Dry run\n") + fmt.Fprintf(out, "Collections selected: %d\n", len(collectionsToRestore)) + fmt.Fprintf(out, "Mode: Dry run\n") } else { - fmt.Printf("Mode: Live restore\n") + fmt.Fprintf(out, "Collections restored: %d\n", collectionsRestored) + fmt.Fprintf(out, "Mode: Live restore\n") + } + fmt.Fprintln(out, "──────────────────────────────────────────────────") + + if !dryRun { + if totalRestored == 0 && totalDocuments > 0 { + return fmt.Errorf("restore failed: none of the %d document(s) in the backup could be restored", totalDocuments) + } + if totalFailed > 0 { + return fmt.Errorf("restore incomplete: %d document(s) failed to restore", totalFailed) + } + if staleRefDocs > 0 { + return fmt.Errorf("restore incomplete: %d document(s) could not be updated to reference the uploaded media", staleRefDocs) + } } - fmt.Println("──────────────────────────────────────────────────") return nil }, } +// isPermissionDenied reports whether err came back as an HTTP 401/403. +// internal/client formats API errors as " (HTTP 403)" when the body +// carries an error envelope and "HTTP 403: " when it does not. +func isPermissionDenied(err error) bool { + if err == nil { + return false + } + msg := err.Error() + for _, marker := range []string{" (HTTP 401)", " (HTTP 403)", "HTTP 401", "HTTP 403"} { + if strings.Contains(msg, marker) { + return true + } + } + return false +} + +// applyMediaReferences applies the media ID mappings to a copy of a restored +// document. It returns the rewritten copy and the number of references changed, +// or a zero count when the document has no media references at all (in which +// case it must not be written back). +func applyMediaReferences(rd restoredDoc, mediaIDMappings map[string]string) (map[string]interface{}, int, error) { + original, err := json.Marshal(rd.body) + if err != nil { + return nil, 0, err + } + var doc map[string]interface{} + if err := json.Unmarshal(original, &doc); err != nil { + return nil, 0, err + } + + count := 0 + if rd.hasSchema && len(rd.schema.Fields) > 0 { + var refCount int + doc, refCount = backup.UpdateReferences(doc, rd.schema, mediaIDMappings) + count += refCount + } + count += backup.DeepUpdateMediaRefs(doc, mediaIDMappings) + count += backup.DeepReplaceMediaIDsInStrings(doc, mediaIDMappings) + if count == 0 { + return nil, 0, nil + } + + rewritten, err := json.Marshal(doc) + if err != nil { + return nil, 0, err + } + if bytes.Equal(original, rewritten) { + return nil, 0, nil + } + return doc, count, nil +} + // extractMediaID extracts the new media ID from an upload response. // Handles various response shapes: {files: [{id}]}, {file: {id}}, {id}, etc. func extractMediaID(result map[string]interface{}) string { diff --git a/cmd/restore_test.go b/cmd/restore_test.go new file mode 100644 index 0000000..b1e35f7 --- /dev/null +++ b/cmd/restore_test.go @@ -0,0 +1,467 @@ +package cmd + +import ( + "archive/zip" + "bytes" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" +) + +// recorder captures every request a fake Trokky instance receives so a test can +// assert on what the restore actually did — in particular that nothing was +// uploaded before the documents were written. +type recorder struct { + mu sync.Mutex + requests []recordedRequest +} + +type recordedRequest struct { + Method string + Path string + Body string +} + +// apiPath is the request path with the instance's "/api" base stripped, so +// tests can assert on "/collections/posts" regardless of how the URL was +// normalised. +func apiPath(req *http.Request) string { + return strings.TrimPrefix(req.URL.Path, "/api") +} + +func (r *recorder) record(req *http.Request) { + body, _ := io.ReadAll(req.Body) + r.mu.Lock() + defer r.mu.Unlock() + r.requests = append(r.requests, recordedRequest{Method: req.Method, Path: apiPath(req), Body: string(body)}) +} + +func (r *recorder) all() []recordedRequest { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]recordedRequest, len(r.requests)) + copy(out, r.requests) + return out +} + +func (r *recorder) countPath(path string) int { + n := 0 + for _, req := range r.all() { + if req.Path == path { + n++ + } + } + return n +} + +const ( + testCollectionsResponse = `{"success":true,"data":{"collections":[{"name":"posts","title":"Posts","fields":[{"name":"title","type":"string"}]}]}}` + testOldMediaID = "media-old-1" + testNewMediaID = "media-new-1" + testNewDocID = "doc-new-1" +) + +// writeBackupZip builds a minimal but realistic v2 backup archive: a manifest, +// one document that references one media file, and that media file. +func writeBackupZip(t *testing.T) string { + t.Helper() + + manifest := map[string]interface{}{ + "version": "2.0", + "timestamp": "2026-01-01T00:00:00Z", + "source": map[string]interface{}{"url": "https://cms.example.com/api"}, + "schemas": []map[string]interface{}{ + { + "name": "posts", + "title": "Posts", + "fields": []map[string]interface{}{{"name": "title", "type": "string"}}, + }, + }, + "dependencyGraph": map[string][]string{}, + "restoreOrder": []string{"posts"}, + "mediaIndex": map[string]interface{}{ + testOldMediaID: map[string]interface{}{ + "filename": "photo.png", + "mimeType": "image/png", + "size": 3, + }, + }, + "statistics": map[string]interface{}{"totalDocuments": 1, "totalMedia": 1}, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + + doc := map[string]interface{}{ + "id": "doc-old-1", + "title": "Example post", + "cover": map[string]interface{}{ + "asset": map[string]interface{}{"_ref": testOldMediaID}, + }, + } + docJSON, err := json.Marshal(doc) + if err != nil { + t.Fatal(err) + } + + path := filepath.Join(t.TempDir(), "backup.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + entries := []struct { + name string + data []byte + }{ + {"manifest.json", manifestJSON}, + {"collections/posts/doc-old-1.json", docJSON}, + {"media/photo.png", []byte("png")}, + } + for _, e := range entries { + w, err := zw.Create(e.name) + if err != nil { + t.Fatal(err) + } + if _, err := w.Write(e.data); err != nil { + t.Fatal(err) + } + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + t.Fatal(err) + } + return path +} + +// runRestore drives the real restore command through cobra with the given args +// and returns its stdout and stderr. +func runRestore(t *testing.T, args ...string) (string, string, error) { + t.Helper() + + // --url/--token take priority over the config file, but point HOME at a temp + // dir so a developer's ~/.trokky/config.yaml can never leak in. + t.Setenv("HOME", t.TempDir()) + t.Setenv("TROKKY_URL", "") + t.Setenv("TROKKY_TOKEN", "") + t.Setenv("TROKKY_INSTANCE", "") + + // Cobra flags are sticky across Execute calls on the shared rootCmd. + for name, def := range map[string]string{ + "collections": "", + "with-dependencies": "false", + "clean": "false", + "overwrite": "false", + "dry-run": "false", + } { + if err := restoreCmd.Flags().Set(name, def); err != nil { + t.Fatal(err) + } + } + + var stdout, stderr bytes.Buffer + rootCmd.SetOut(&stdout) + rootCmd.SetErr(&stderr) + defer func() { + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + }() + + rootCmd.SetArgs(append([]string{"restore"}, args...)) + + // Anything printed straight to the process streams counts as output too. + var err error + rawOut, rawErr := captureProcessOutput(t, func() { + err = rootCmd.Execute() + }) + + return stdout.String() + rawOut, stderr.String() + rawErr, err +} + +// captureProcessOutput runs fn with os.Stdout/os.Stderr redirected to pipes and +// returns what was written to each. +func captureProcessOutput(t *testing.T, fn func()) (string, string) { + t.Helper() + + outR, outW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + errR, errW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + + origOut, origErr := os.Stdout, os.Stderr + os.Stdout, os.Stderr = outW, errW + + var wg sync.WaitGroup + var outBuf, errBuf bytes.Buffer + wg.Add(2) + go func() { defer wg.Done(); io.Copy(&outBuf, outR) }() + go func() { defer wg.Done(); io.Copy(&errBuf, errR) }() + + fn() + + os.Stdout, os.Stderr = origOut, origErr + outW.Close() + errW.Close() + wg.Wait() + outR.Close() + errR.Close() + + return outBuf.String(), errBuf.String() +} + +// A token with content:read/media:read but no content:write must be rejected +// before a single media file is uploaded — uploading first mints new media IDs +// and orphans them against documents that were never written. +func TestRestoreAbortsBeforeUploadingMediaWhenDocumentWritesAreForbidden(t *testing.T) { + rec := &recorder{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + if r.Method == http.MethodGet && apiPath(r) == "/collections" { + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, testCollectionsResponse) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + io.WriteString(w, `{"success":false,"error":{"message":"content:write scope required"}}`) + })) + defer server.Close() + + stdout, _, err := runRestore(t, "--input", writeBackupZip(t), "--url", server.URL+"/api", "--token", "read-only", "-q") + + want := "token lacks content:write (required to restore documents): " + if err == nil { + t.Error("expected an error for a token that cannot write documents") + } else if !strings.HasPrefix(err.Error(), want) { + t.Errorf("error = %q, want it to start with %q", err.Error(), want) + } + if n := rec.countPath("/media/upload"); n != 0 { + t.Errorf("%d request(s) hit /media/upload; the restore must abort before uploading anything", n) + } + if strings.Contains(stdout, "Restore completed") { + t.Errorf("output claims success after a failed restore:\n%s", stdout) + } +} + +// A restore where every document write fails must exit non-zero and must not +// claim to have completed. +func TestRestoreFailedDocumentWritesExitNonZero(t *testing.T) { + rec := &recorder{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + if r.Method == http.MethodGet && apiPath(r) == "/collections" { + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, testCollectionsResponse) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + io.WriteString(w, `{"success":false,"error":{"message":"boom"}}`) + })) + defer server.Close() + + stdout, _, err := runRestore(t, "--input", writeBackupZip(t), "--url", server.URL+"/api", "--token", "t", "-q") + + if err == nil { + t.Error("expected a non-zero exit when no document could be restored") + } else if !strings.Contains(err.Error(), "restore failed") { + t.Errorf("error = %q, want it to mention that the restore failed", err.Error()) + } + if strings.Contains(stdout, "Restore completed") { + t.Errorf("output claims success after a failed restore:\n%s", stdout) + } + if !strings.Contains(stdout, "Documents restored: 0") { + t.Errorf("summary does not report 0 documents restored:\n%s", stdout) + } + if n := rec.countPath("/media/upload"); n != 0 { + t.Errorf("%d media upload(s) happened even though no document was restored", n) + } +} + +// Counters must describe work that actually landed on the server: when the +// reference-fix pass is rejected, "References updated" stays at 0 and the user +// is warned that the uploaded media is orphaned. +func TestRestoreCountersReportOnlyRealWork(t *testing.T) { + rec := &recorder{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && apiPath(r) == "/collections": + io.WriteString(w, testCollectionsResponse) + case r.Method == http.MethodPost && apiPath(r) == "/collections/posts": + io.WriteString(w, `{"success":true,"data":{"id":"`+testNewDocID+`"}}`) + case r.Method == http.MethodPost && apiPath(r) == "/media/upload": + io.WriteString(w, `{"success":true,"data":{"files":[{"id":"`+testNewMediaID+`"}]}}`) + default: + w.WriteHeader(http.StatusForbidden) + io.WriteString(w, `{"success":false,"error":{"message":"content:write scope required"}}`) + } + })) + defer server.Close() + + stdout, stderr, err := runRestore(t, "--input", writeBackupZip(t), "--url", server.URL+"/api", "--token", "t", "-q") + + if err == nil { + t.Error("expected a non-zero exit when media references could not be written") + } + if !strings.Contains(stdout, "References updated: 0") { + t.Errorf("References updated must count only documents that were rewritten:\n%s", stdout) + } + if !strings.Contains(stdout, "Media restored: 1") { + t.Errorf("summary does not report the uploaded media file:\n%s", stdout) + } + if !strings.Contains(stdout, "Documents restored: 1") { + t.Errorf("summary does not report the restored document:\n%s", stdout) + } + if strings.Contains(stdout, "Restore completed") { + t.Errorf("output claims success although the documents still point at the old media IDs:\n%s", stdout) + } + if !strings.Contains(stderr, "ORPHANED MEDIA") { + t.Errorf("no orphaned-media warning on stderr:\n%s", stderr) + } + if !strings.Contains(stderr, "1 media file(s) were uploaded under new IDs") { + t.Errorf("orphaned-media warning does not name the counts:\n%s", stderr) + } +} + +func TestRestoreHappyPathReportsTrueCounts(t *testing.T) { + rec := &recorder{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && apiPath(r) == "/collections": + io.WriteString(w, testCollectionsResponse) + case r.Method == http.MethodPost && apiPath(r) == "/collections/posts": + io.WriteString(w, `{"success":true,"data":{"id":"`+testNewDocID+`"}}`) + case r.Method == http.MethodPost && apiPath(r) == "/media/upload": + io.WriteString(w, `{"success":true,"data":{"files":[{"id":"`+testNewMediaID+`"}]}}`) + case r.Method == http.MethodPut && apiPath(r) == "/collections/posts/"+testNewDocID: + io.WriteString(w, `{"success":true,"data":{"id":"`+testNewDocID+`"}}`) + default: + w.WriteHeader(http.StatusNotFound) + io.WriteString(w, `{"success":false,"error":{"message":"unexpected `+r.Method+` `+apiPath(r)+`"}}`) + } + })) + defer server.Close() + + stdout, stderr, err := runRestore(t, "--input", writeBackupZip(t), "--url", server.URL+"/api", "--token", "t", "-q") + if err != nil { + t.Fatalf("restore failed: %v\nstdout:\n%s\nstderr:\n%s", err, stdout, stderr) + } + + for _, want := range []string{ + "Restore completed", + "Documents restored: 1", + "Media restored: 1", + "References updated: 1", + "Collections restored: 1", + } { + if !strings.Contains(stdout, want) { + t.Errorf("output is missing %q:\n%s", want, stdout) + } + } + if strings.Contains(stderr, "ORPHANED MEDIA") { + t.Errorf("orphaned-media warning on a clean run:\n%s", stderr) + } + + // The document must be POSTed before the media is uploaded, and the + // reference-fix PUT must carry the new media ID. + var order []string + var putBody string + for _, req := range rec.all() { + if req.Method == http.MethodPost || req.Method == http.MethodPut { + order = append(order, req.Method+" "+req.Path) + } + if req.Method == http.MethodPut && req.Path == "/collections/posts/"+testNewDocID { + putBody = req.Body + } + } + wantOrder := []string{ + "POST /collections/posts", + "POST /media/upload", + "PUT /collections/posts/" + testNewDocID, + } + if strings.Join(order, ", ") != strings.Join(wantOrder, ", ") { + t.Errorf("write order = %v, want %v", order, wantOrder) + } + if !strings.Contains(putBody, testNewMediaID) { + t.Errorf("reference-fix PUT body does not contain the new media ID: %s", putBody) + } + if strings.Contains(putBody, testOldMediaID) { + t.Errorf("reference-fix PUT body still contains the old media ID: %s", putBody) + } +} + +func TestRestoreDryRunMakesNoWrites(t *testing.T) { + rec := &recorder{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodGet && apiPath(r) == "/collections" { + io.WriteString(w, testCollectionsResponse) + return + } + w.WriteHeader(http.StatusInternalServerError) + io.WriteString(w, `{"success":false,"error":{"message":"a dry run must not write"}}`) + })) + defer server.Close() + + stdout, _, err := runRestore(t, "--input", writeBackupZip(t), "--url", server.URL+"/api", "--token", "t", "--dry-run", "-q") + if err != nil { + t.Fatalf("dry run failed: %v\n%s", err, stdout) + } + for _, want := range []string{ + "[DRY RUN] Would restore 1 media file(s)", + "[DRY RUN] Would restore 1 document(s) to posts", + "Dry run completed - no changes made", + } { + if !strings.Contains(stdout, want) { + t.Errorf("output is missing %q:\n%s", want, stdout) + } + } + for _, req := range rec.all() { + if req.Method != http.MethodGet { + t.Errorf("dry run issued a %s to %s", req.Method, req.Path) + } + } +} + +func TestIsPermissionDenied(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + {"envelope 403", errors.New("content:write scope required (HTTP 403)"), true}, + {"envelope 401", errors.New("invalid token (HTTP 401)"), true}, + {"raw 403", errors.New("HTTP 403: Forbidden"), true}, + {"raw 401", errors.New("HTTP 401: Unauthorized"), true}, + {"upload 403", errors.New("upload failed: HTTP 403: Forbidden"), true}, + {"server error", errors.New("boom (HTTP 500)"), false}, + {"not found", errors.New("HTTP 404: not found"), false}, + {"conflict", errors.New("document already exists (HTTP 409)"), false}, + {"transport error", errors.New("request failed: connection refused"), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isPermissionDenied(tt.err); got != tt.want { + t.Errorf("isPermissionDenied(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index a0dd933..6b31e16 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "net/url" "os" "path/filepath" "strings" @@ -237,13 +238,40 @@ func SetDefaultInstance(name string) (bool, error) { return true, Save(cfg) } -// NormalizeBaseURL trims trailing slashes and ensures the URL ends with /api. +// NormalizeBaseURL trims trailing slashes and ensures the URL points at the +// Trokky API root, so that "http://cms.example.com:3210" and +// "http://cms.example.com:3210/api/" both resolve to +// "http://cms.example.com:3210/api". +// +// Rules: +// - Trailing slashes are always trimmed. +// - If the path already contains an "api" segment the URL is left alone. +// This keeps the function idempotent for ".../api" and ".../api/", and +// avoids mangling a URL that is already mounted deeper than the API root +// (e.g. ".../api/v1" or ".../backend/api" stay as they are). +// - Otherwise "/api" is appended. +// +// An empty string is returned unchanged. func NormalizeBaseURL(rawURL string) string { - u := strings.TrimRight(rawURL, "/") - if !strings.HasSuffix(u, "/api") { - u += "/api" + trimmed := strings.TrimRight(rawURL, "/") + if trimmed == "" { + return trimmed } - return u + + if u, err := url.Parse(trimmed); err == nil && u.Host != "" { + for _, segment := range strings.Split(u.Path, "/") { + if segment == "api" { + return trimmed + } + } + return trimmed + "/api" + } + + // Unparseable or host-less input: fall back to a plain suffix check. + if strings.HasSuffix(trimmed, "/api") { + return trimmed + } + return trimmed + "/api" } // MaskToken masks a token for display, showing first 4 and last 4 characters. @@ -262,7 +290,7 @@ func ResolveCredentials(opts ResolveOptions) (*ResolvedCredentials, error) { // Priority 1: CLI flags if opts.URL != "" && opts.Token != "" { return &ResolvedCredentials{ - URL: opts.URL, + URL: NormalizeBaseURL(opts.URL), Token: opts.Token, Source: "cli", }, nil @@ -273,7 +301,7 @@ func ResolveCredentials(opts ResolveOptions) (*ResolvedCredentials, error) { envToken := os.Getenv(EnvToken) if envURL != "" && envToken != "" { return &ResolvedCredentials{ - URL: envURL, + URL: NormalizeBaseURL(envURL), Token: envToken, Source: "env", }, nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2dc8300..6f5be57 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -496,8 +496,111 @@ func TestMaskToken_JustAboveThreshold(t *testing.T) { } } +// --- NormalizeBaseURL --- + +func TestNormalizeBaseURL(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"appends missing /api", "http://cms.example.com:3210", "http://cms.example.com:3210/api"}, + {"already has /api", "http://cms.example.com:3210/api", "http://cms.example.com:3210/api"}, + {"trailing slash", "http://cms.example.com:3210/", "http://cms.example.com:3210/api"}, + {"trailing slash after /api", "http://cms.example.com:3210/api/", "http://cms.example.com:3210/api"}, + {"multiple trailing slashes", "http://cms.example.com:3210///", "http://cms.example.com:3210/api"}, + {"https with subpath", "https://cms.example.com/cms", "https://cms.example.com/cms/api"}, + {"api segment in subpath", "https://cms.example.com/backend/api", "https://cms.example.com/backend/api"}, + {"versioned api path left alone", "https://cms.example.com/api/v1", "https://cms.example.com/api/v1"}, + {"api in hostname only", "https://api.example.com", "https://api.example.com/api"}, + {"empty string", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizeBaseURL(tt.in) + if got != tt.want { + t.Fatalf("NormalizeBaseURL(%q) = %q, want %q", tt.in, got, tt.want) + } + // Normalizing again must not change the result. + if again := NormalizeBaseURL(got); again != got { + t.Fatalf("NormalizeBaseURL is not idempotent: %q -> %q -> %q", tt.in, got, again) + } + }) + } +} + // --- ResolveCredentials --- +func TestResolveCredentials_NormalizesURL(t *testing.T) { + tests := []struct { + name string + url string + want string + }{ + {"flag without /api", "http://cms.example.com:3210", "http://cms.example.com:3210/api"}, + {"flag with /api", "http://cms.example.com:3210/api", "http://cms.example.com:3210/api"}, + {"flag with trailing slash", "http://cms.example.com:3210/", "http://cms.example.com:3210/api"}, + {"flag with /api and trailing slash", "http://cms.example.com:3210/api/", "http://cms.example.com:3210/api"}, + {"flag mounted under a subpath", "https://cms.example.com/backend/api", "https://cms.example.com/backend/api"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearEnvVars(t) + + creds, err := ResolveCredentials(ResolveOptions{URL: tt.url, Token: "flag-token"}) + if err != nil { + t.Fatal(err) + } + if creds == nil { + t.Fatal("expected credentials") + } + if creds.Source != "cli" { + t.Fatalf("Source = %q, want 'cli'", creds.Source) + } + if creds.URL != tt.want { + t.Fatalf("URL = %q, want %q", creds.URL, tt.want) + } + }) + } +} + +func TestResolveCredentials_NormalizesEnvURL(t *testing.T) { + tests := []struct { + name string + url string + want string + }{ + {"env without /api", "http://cms.example.com:3210", "http://cms.example.com:3210/api"}, + {"env with /api", "http://cms.example.com:3210/api", "http://cms.example.com:3210/api"}, + {"env with trailing slash", "http://cms.example.com:3210/", "http://cms.example.com:3210/api"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + overrideHome(t) + clearEnvVars(t) + t.Setenv(EnvURL, tt.url) + t.Setenv(EnvToken, "env-token") + + creds, err := ResolveCredentials(ResolveOptions{}) + if err != nil { + t.Fatal(err) + } + if creds == nil { + t.Fatal("expected credentials from env") + } + if creds.Source != "env" { + t.Fatalf("Source = %q, want 'env'", creds.Source) + } + if creds.URL != tt.want { + t.Fatalf("URL = %q, want %q", creds.URL, tt.want) + } + }) + } +} + func TestResolveCredentials_CLIFlags(t *testing.T) { clearEnvVars(t) @@ -514,7 +617,8 @@ func TestResolveCredentials_CLIFlags(t *testing.T) { if creds.Source != "cli" { t.Fatalf("Source = %q, want 'cli'", creds.Source) } - if creds.URL != "http://from-flag" { + // The CLI URL is normalized to the API root (see TestResolveCredentials_NormalizesURL). + if creds.URL != "http://from-flag/api" { t.Fatalf("URL = %q", creds.URL) } if creds.Token != "flag-token" { @@ -549,7 +653,8 @@ func TestResolveCredentials_EnvVars(t *testing.T) { if creds.Source != "env" { t.Fatalf("Source = %q, want 'env'", creds.Source) } - if creds.URL != "http://from-env" { + // The env URL is normalized to the API root (see TestResolveCredentials_NormalizesURL). + if creds.URL != "http://from-env/api" { t.Fatalf("URL = %q", creds.URL) } } @@ -667,7 +772,7 @@ func TestRequireCredentials_Success(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if creds.URL != "http://ok" { + if creds.URL != "http://ok/api" { t.Fatalf("URL = %q", creds.URL) } }