diff --git a/.env.appview.example b/.env.appview.example index 6de686a..0a11c5b 100644 --- a/.env.appview.example +++ b/.env.appview.example @@ -61,6 +61,11 @@ ATCR_UI_ENABLED=true # Default: /var/lib/atcr/ui.db # ATCR_UI_DATABASE_PATH=/var/lib/atcr/ui.db +# Skip database migrations on startup (default: false) +# Set to "true" to skip running migrations (useful for tests or fresh databases) +# Production: Keep as "false" to ensure migrations are applied +SKIP_DB_MIGRATIONS=false + # ============================================================================== # Logging Configuration # ============================================================================== diff --git a/cmd/appview/serve.go b/cmd/appview/serve.go index 75424b7..f894a83 100644 --- a/cmd/appview/serve.go +++ b/cmd/appview/serve.go @@ -74,7 +74,7 @@ func serveRegistry(cmd *cobra.Command, args []string) error { // Initialize UI database first (required for all stores) slog.Info("Initializing UI database", "path", cfg.UI.DatabasePath) - uiDatabase, uiReadOnlyDB, uiSessionStore := db.InitializeDatabase(cfg.UI.Enabled, cfg.UI.DatabasePath) + uiDatabase, uiReadOnlyDB, uiSessionStore := db.InitializeDatabase(cfg.UI.Enabled, cfg.UI.DatabasePath, cfg.UI.SkipDBMigrations) if uiDatabase == nil { return fmt.Errorf("failed to initialize UI database - required for session storage") } diff --git a/deploy/.env.prod.template b/deploy/.env.prod.template index 0e6aed0..81a68a2 100644 --- a/deploy/.env.prod.template +++ b/deploy/.env.prod.template @@ -155,6 +155,12 @@ ATCR_TOKEN_EXPIRATION=300 # Default: true ATCR_UI_ENABLED=true +# Skip database migrations on startup +# Default: false (migrations are applied on startup) +# Set to "true" only for testing or when migrations are managed externally +# Production: Keep as "false" to ensure migrations are applied +SKIP_DB_MIGRATIONS=false + # ============================================================================== # Logging Configuration # ============================================================================== diff --git a/docs/ANNOTATIONS_REFACTOR.md b/docs/ANNOTATIONS_REFACTOR.md deleted file mode 100644 index dd20a7b..0000000 --- a/docs/ANNOTATIONS_REFACTOR.md +++ /dev/null @@ -1,577 +0,0 @@ -# Annotations Table Refactoring - -## Overview - -Refactor manifest annotations from individual columns (`title`, `description`, `source_url`, etc.) to a normalized key-value table. This enables flexible annotation storage without schema changes for new OCI annotations. - -## Motivation - -**Current Problems:** -- Each new annotation (e.g., `org.opencontainers.image.version`) requires schema change -- Many NULL columns in manifests table -- Rigid schema doesn't match OCI's flexible annotation model - -**Benefits:** -- ✅ Add any annotation without code/schema changes -- ✅ Normalized database design -- ✅ Easy to query "all repos with annotation X" -- ✅ Simple queries (no joins needed for repository pages) - -## Database Schema Changes - -### 1. New Table: `repository_annotations` - -```sql -CREATE TABLE IF NOT EXISTS repository_annotations ( - did TEXT NOT NULL, - repository TEXT NOT NULL, - key TEXT NOT NULL, - value TEXT NOT NULL, - updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - PRIMARY KEY(did, repository, key), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX IF NOT EXISTS idx_repository_annotations_did_repo ON repository_annotations(did, repository); -CREATE INDEX IF NOT EXISTS idx_repository_annotations_key ON repository_annotations(key); -``` - -**Key Design Decisions:** -- Primary key: `(did, repository, key)` - one value per annotation per repository -- No `manifest_id` foreign key - annotations are repository-level, not manifest-level -- `updated_at` - track when annotation was last updated (from most recent manifest) -- Stored at repository level because that's where they're displayed - -### 2. Drop Columns from `manifests` Table - -Remove these columns (migration will preserve data by copying to annotations table): -- `title` -- `description` -- `source_url` -- `documentation_url` -- `licenses` -- `icon_url` -- `readme_url` -- `version` - -Keep only core manifest metadata: -- `id`, `did`, `repository`, `digest` -- `hold_endpoint`, `schema_version`, `media_type` -- `config_digest`, `config_size` -- `created_at` - -## Migration Strategy - -There is no need to migrate data to this new table via sql. on startup, backfill will re-populate the new table with existing annotations. - -## Code Changes - -### 1. Database Helper Functions - -**New file: `pkg/appview/db/annotations.go`** - -```go -package db - -import ( - "database/sql" - "time" -) - -// GetRepositoryAnnotations retrieves all annotations for a repository -func GetRepositoryAnnotations(db *sql.DB, did, repository string) (map[string]string, error) { - rows, err := db.Query(` - SELECT key, value - FROM repository_annotations - WHERE did = ? AND repository = ? - `, did, repository) - if err != nil { - return nil, err - } - defer rows.Close() - - annotations := make(map[string]string) - for rows.Next() { - var key, value string - if err := rows.Scan(&key, &value); err != nil { - return nil, err - } - annotations[key] = value - } - - return annotations, rows.Err() -} - -// UpsertRepositoryAnnotations replaces all annotations for a repository -// Only called when manifest has at least one non-empty annotation -func UpsertRepositoryAnnotations(db *sql.DB, did, repository string, annotations map[string]string) error { - tx, err := db.Begin() - if err != nil { - return err - } - defer tx.Rollback() - - // Delete existing annotations - _, err = tx.Exec(` - DELETE FROM repository_annotations - WHERE did = ? AND repository = ? - `, did, repository) - if err != nil { - return err - } - - // Insert new annotations - stmt, err := tx.Prepare(` - INSERT INTO repository_annotations (did, repository, key, value, updated_at) - VALUES (?, ?, ?, ?, ?) - `) - if err != nil { - return err - } - defer stmt.Close() - - now := time.Now() - for key, value := range annotations { - _, err = stmt.Exec(did, repository, key, value, now) - if err != nil { - return err - } - } - - return tx.Commit() -} - -// DeleteRepositoryAnnotations removes all annotations for a repository -func DeleteRepositoryAnnotations(db *sql.DB, did, repository string) error { - _, err := db.Exec(` - DELETE FROM repository_annotations - WHERE did = ? AND repository = ? - `, did, repository) - return err -} -``` - -### 2. Update Backfill Worker - -**File: `pkg/appview/jetstream/backfill.go`** - -In `processManifestRecord()` function, after extracting annotations: - -```go -// Extract OCI annotations from manifest -var title, description, sourceURL, documentationURL, licenses, iconURL, readmeURL string -if manifestRecord.Annotations != nil { - title = manifestRecord.Annotations["org.opencontainers.image.title"] - description = manifestRecord.Annotations["org.opencontainers.image.description"] - sourceURL = manifestRecord.Annotations["org.opencontainers.image.source"] - documentationURL = manifestRecord.Annotations["org.opencontainers.image.documentation"] - licenses = manifestRecord.Annotations["org.opencontainers.image.licenses"] - iconURL = manifestRecord.Annotations["io.atcr.icon"] - readmeURL = manifestRecord.Annotations["io.atcr.readme"] -} - -// Prepare manifest for insertion (WITHOUT annotation fields) -manifest := &db.Manifest{ - DID: did, - Repository: manifestRecord.Repository, - Digest: manifestRecord.Digest, - MediaType: manifestRecord.MediaType, - SchemaVersion: manifestRecord.SchemaVersion, - HoldEndpoint: manifestRecord.HoldEndpoint, - CreatedAt: manifestRecord.CreatedAt, - // NO annotation fields -} - -// Set config fields only for image manifests (not manifest lists) -if !isManifestList && manifestRecord.Config != nil { - manifest.ConfigDigest = manifestRecord.Config.Digest - manifest.ConfigSize = manifestRecord.Config.Size -} - -// Insert manifest -manifestID, err := db.InsertManifest(b.db, manifest) -if err != nil { - return fmt.Errorf("failed to insert manifest: %w", err) -} - -// Update repository annotations ONLY if manifest has at least one non-empty annotation -if manifestRecord.Annotations != nil { - hasData := false - for _, value := range manifestRecord.Annotations { - if value != "" { - hasData = true - break - } - } - - if hasData { - // Replace all annotations for this repository - err = db.UpsertRepositoryAnnotations(b.db, did, manifestRecord.Repository, manifestRecord.Annotations) - if err != nil { - return fmt.Errorf("failed to upsert annotations: %w", err) - } - } -} -``` - -### 3. Update Jetstream Worker - -**File: `pkg/appview/jetstream/worker.go`** - -Same changes as backfill - in `processManifestCommit()` function: - -```go -// Extract OCI annotations from manifest -var title, description, sourceURL, documentationURL, licenses, iconURL, readmeURL string -if manifestRecord.Annotations != nil { - title = manifestRecord.Annotations["org.opencontainers.image.title"] - description = manifestRecord.Annotations["org.opencontainers.image.description"] - sourceURL = manifestRecord.Annotations["org.opencontainers.image.source"] - documentationURL = manifestRecord.Annotations["org.opencontainers.image.documentation"] - licenses = manifestRecord.Annotations["org.opencontainers.image.licenses"] - iconURL = manifestRecord.Annotations["io.atcr.icon"] - readmeURL = manifestRecord.Annotations["io.atcr.readme"] -} - -// Prepare manifest for insertion (WITHOUT annotation fields) -manifest := &db.Manifest{ - DID: commit.DID, - Repository: manifestRecord.Repository, - Digest: manifestRecord.Digest, - MediaType: manifestRecord.MediaType, - SchemaVersion: manifestRecord.SchemaVersion, - HoldEndpoint: manifestRecord.HoldEndpoint, - CreatedAt: manifestRecord.CreatedAt, - // NO annotation fields -} - -// Set config fields only for image manifests (not manifest lists) -if !isManifestList && manifestRecord.Config != nil { - manifest.ConfigDigest = manifestRecord.Config.Digest - manifest.ConfigSize = manifestRecord.Config.Size -} - -// Insert manifest -manifestID, err := db.InsertManifest(w.db, manifest) -if err != nil { - return fmt.Errorf("failed to insert manifest: %w", err) -} - -// Update repository annotations ONLY if manifest has at least one non-empty annotation -if manifestRecord.Annotations != nil { - hasData := false - for _, value := range manifestRecord.Annotations { - if value != "" { - hasData = true - break - } - } - - if hasData { - // Replace all annotations for this repository - err = db.UpsertRepositoryAnnotations(w.db, commit.DID, manifestRecord.Repository, manifestRecord.Annotations) - if err != nil { - return fmt.Errorf("failed to upsert annotations: %w", err) - } - } -} -``` - -### 4. Update Database Queries - -**File: `pkg/appview/db/queries.go`** - -Replace `GetRepositoryMetadata()` function: - -```go -// GetRepositoryMetadata retrieves metadata for a repository from annotations table -func GetRepositoryMetadata(db *sql.DB, did string, repository string) (title, description, sourceURL, documentationURL, licenses, iconURL, readmeURL, version string, err error) { - annotations, err := GetRepositoryAnnotations(db, did, repository) - if err != nil { - return "", "", "", "", "", "", "", "", err - } - - title = annotations["org.opencontainers.image.title"] - description = annotations["org.opencontainers.image.description"] - sourceURL = annotations["org.opencontainers.image.source"] - documentationURL = annotations["org.opencontainers.image.documentation"] - licenses = annotations["org.opencontainers.image.licenses"] - iconURL = annotations["io.atcr.icon"] - readmeURL = annotations["io.atcr.readme"] - version = annotations["org.opencontainers.image.version"] - - return title, description, sourceURL, documentationURL, licenses, iconURL, readmeURL, version, nil -} -``` - -Update `InsertManifest()` to remove annotation columns: - -```go -func InsertManifest(db *sql.DB, manifest *Manifest) (int64, error) { - _, err := db.Exec(` - INSERT INTO manifests - (did, repository, digest, hold_endpoint, schema_version, media_type, - config_digest, config_size, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(did, repository, digest) DO UPDATE SET - hold_endpoint = excluded.hold_endpoint, - schema_version = excluded.schema_version, - media_type = excluded.media_type, - config_digest = excluded.config_digest, - config_size = excluded.config_size - `, manifest.DID, manifest.Repository, manifest.Digest, manifest.HoldEndpoint, - manifest.SchemaVersion, manifest.MediaType, manifest.ConfigDigest, - manifest.ConfigSize, manifest.CreatedAt) - - if err != nil { - return 0, err - } - - // Query for the ID (works for both insert and update) - var id int64 - err = db.QueryRow(` - SELECT id FROM manifests - WHERE did = ? AND repository = ? AND digest = ? - `, manifest.DID, manifest.Repository, manifest.Digest).Scan(&id) - - if err != nil { - return 0, fmt.Errorf("failed to get manifest ID after upsert: %w", err) - } - - return id, nil -} -``` - -Similar updates needed for: -- `GetUserRepositories()` - fetch annotations separately and populate Repository struct -- `GetRecentPushes()` - join with annotations or fetch separately -- `SearchPushes()` - can now search annotations table directly - -### 5. Update Models - -**File: `pkg/appview/db/models.go`** - -Remove annotation fields from `Manifest` struct: - -```go -type Manifest struct { - ID int64 - DID string - Repository string - Digest string - HoldEndpoint string - SchemaVersion int - MediaType string - ConfigDigest string - ConfigSize int64 - CreatedAt time.Time - // Removed: Title, Description, SourceURL, DocumentationURL, Licenses, IconURL, ReadmeURL -} -``` - -Keep annotation fields on `Repository` struct (populated from annotations table): - -```go -type Repository struct { - Name string - TagCount int - ManifestCount int - LastPush time.Time - Tags []Tag - Manifests []Manifest - Title string - Description string - SourceURL string - DocumentationURL string - Licenses string - IconURL string - ReadmeURL string - Version string // NEW -} -``` - -### 6. Update Schema.sql - -**File: `pkg/appview/db/schema.sql`** - -Add new table: - -```sql -CREATE TABLE IF NOT EXISTS repository_annotations ( - did TEXT NOT NULL, - repository TEXT NOT NULL, - key TEXT NOT NULL, - value TEXT NOT NULL, - updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - PRIMARY KEY(did, repository, key), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX IF NOT EXISTS idx_repository_annotations_did_repo ON repository_annotations(did, repository); -CREATE INDEX IF NOT EXISTS idx_repository_annotations_key ON repository_annotations(key); -``` - -Update manifests table (remove annotation columns): - -```sql -CREATE TABLE IF NOT EXISTS manifests ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - did TEXT NOT NULL, - repository TEXT NOT NULL, - digest TEXT NOT NULL, - hold_endpoint TEXT NOT NULL, - schema_version INTEGER NOT NULL, - media_type TEXT NOT NULL, - config_digest TEXT, - config_size INTEGER, - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, digest), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -``` - -## Update Logic Summary - -**Key Decision: Only update annotations when manifest has data** - -``` -For each manifest processed (backfill or jetstream): - 1. Parse manifest.Annotations map - 2. Check if ANY annotation has non-empty value - 3. IF hasData: - DELETE all annotations for (did, repository) - INSERT all annotations from manifest (including empty ones) - ELSE: - SKIP (don't touch existing annotations) -``` - -**Why this works:** -- Manifest lists have no annotations or all empty → skip, preserve existing -- Platform manifests have real data → replace everything -- Removing annotation from Dockerfile → it's gone (not in new INSERT) -- Can't accidentally clear data (need at least one non-empty value) - -## UI/Template Changes - -### Handler Updates - -**File: `pkg/appview/handlers/repository.go`** - -Update the handler to include version: - -```go -// Fetch repository metadata from annotations -title, description, sourceURL, documentationURL, licenses, iconURL, readmeURL, version, err := db.GetRepositoryMetadata(h.DB, owner.DID, repository) -if err != nil { - log.Printf("Failed to fetch repository metadata: %v", err) - // Continue without metadata on error -} else { - repo.Title = title - repo.Description = description - repo.SourceURL = sourceURL - repo.DocumentationURL = documentationURL - repo.Licenses = licenses - repo.IconURL = iconURL - repo.ReadmeURL = readmeURL - repo.Version = version // NEW -} -``` - -### Template Updates - -**File: `pkg/appview/templates/pages/repository.html`** - -Update the metadata section condition to include version: - -```html - -{{ if or .Repository.Licenses .Repository.SourceURL .Repository.DocumentationURL .Repository.Version }} -
- - {{ if .Repository.Version }} - - {{ .Repository.Version }} - - {{ end }} - - - {{ if .Repository.Licenses }} - {{ range parseLicenses .Repository.Licenses }} - {{ if .IsValid }} - - {{ .SPDXID }} - - {{ else }} - - {{ .Name }} - - {{ end }} - {{ end }} - {{ end }} - - - {{ if .Repository.SourceURL }} - - Source - - {{ end }} - - - {{ if .Repository.DocumentationURL }} - - Documentation - - {{ end }} -
-{{ end }} -``` - -### CSS Updates - -**File: `pkg/appview/static/css/style.css`** - -Add styling for version badge (different color from license badge): - -```css -.version-badge { - background: #0969da; /* GitHub blue */ - color: white; - padding: 0.25rem 0.5rem; - border-radius: 0.25rem; - font-size: 0.875rem; - font-weight: 500; - display: inline-block; -} -``` - -### Data Flow Summary - -**Before refactor:** -``` -DB columns → GetRepositoryMetadata() → Handler assigns to Repository struct → Template displays -``` - -**After refactor:** -``` -annotations table → GetRepositoryAnnotations() → GetRepositoryMetadata() extracts known fields → -Handler assigns to Repository struct → Template displays (same as before) -``` - -**Key point:** Templates still access `.Repository.Title`, `.Repository.Version`, etc. - the source just changed from DB columns to annotations table. The abstraction layer hides this complexity. - -## Benefits Recap - -1. **Flexible**: Support any OCI annotation without code changes -2. **Clean**: No NULL columns in manifests table -3. **Simple queries**: `SELECT * FROM repository_annotations WHERE did=? AND repo=?` -4. **Safe updates**: Only update when manifest has data -5. **Natural deletion**: Remove annotation from Dockerfile → it's deleted on next push -6. **Extensible**: Future features (annotation search, filtering) are trivial - -## Testing Checklist - -After migration: -- [ ] Verify existing repositories show annotations correctly -- [ ] Push new manifest with annotations → updates correctly -- [ ] Push manifest list → doesn't clear annotations -- [ ] Remove annotation from Dockerfile and push → annotation deleted -- [ ] Backfill re-run → annotations repopulated correctly -- [ ] Search still works (if implemented) diff --git a/docs/APPVIEW-UI-IMPLEMENTATION.md b/docs/APPVIEW-UI-IMPLEMENTATION.md deleted file mode 100644 index f84659f..0000000 --- a/docs/APPVIEW-UI-IMPLEMENTATION.md +++ /dev/null @@ -1,1827 +0,0 @@ -# ATCR AppView UI - Implementation Guide - -This document provides step-by-step implementation details for building the ATCR web UI using **html/template + HTMX**. - -## Tech Stack (Finalized) - -- **Backend:** Go (existing AppView) -- **Templates:** `html/template` (standard library) -- **Interactivity:** HTMX (~14KB) + Alpine.js (~15KB, optional) -- **Database:** SQLite (firehose cache) -- **Styling:** Simple CSS or Tailwind (TBD) -- **Authentication:** OAuth (existing implementation) - -## Project Structure - -``` -cmd/appview/ -├── main.go # Add AppView routes here - -pkg/appview/ -├── appview.go # Main AppView setup, embed directives -├── handlers/ # HTTP handlers -│ ├── home.go # Front page (firehose) -│ ├── settings.go # Settings page -│ ├── images.go # Personal images page -│ └── auth.go # Login/logout handlers -├── db/ # Database layer -│ ├── schema.go # SQLite schema -│ ├── queries.go # DB queries -│ └── models.go # Data models -├── firehose/ # Firehose worker -│ ├── worker.go # Background worker -│ └── jetstream.go # Jetstream client -├── middleware/ # HTTP middleware -│ ├── auth.go # Session auth -│ └── csrf.go # CSRF protection -├── session/ # Session management -│ └── session.go # Session store -├── templates/ # HTML templates (embedded) -│ ├── layouts/ -│ │ └── base.html # Base layout -│ ├── components/ -│ │ ├── nav.html # Navigation bar -│ │ └── modal.html # Modal dialogs -│ ├── pages/ -│ │ ├── home.html # Front page -│ │ ├── settings.html # Settings page -│ │ └── images.html # Personal images -│ └── partials/ # HTMX partials -│ ├── push-list.html # Push list partial -│ └── tag-row.html # Tag row partial -└── static/ # Static assets (embedded) - ├── css/ - │ └── style.css - └── js/ - └── app.js # Minimal JS (clipboard, etc.) -``` - -## Step 1: Embed Setup - -### Main AppView Package - -**pkg/appview/appview.go:** - -```go -package appview - -import ( - "embed" - "html/template" - "io/fs" - "net/http" -) - -//go:embed templates/*.html templates/**/*.html -var templatesFS embed.FS - -//go:embed static/* -var staticFS embed.FS - -// Templates returns parsed templates -func Templates() (*template.Template, error) { - return template.ParseFS(templatesFS, "templates/**/*.html") -} - -// StaticHandler returns HTTP handler for static files -func StaticHandler() http.Handler { - sub, _ := fs.Sub(staticFS, "static") - return http.FileServer(http.FS(sub)) -} -``` - -## Step 2: Database Setup - -### Create Schema - -**pkg/appview/db/schema.go:** - -```go -package db - -import ( - "database/sql" - _ "github.com/mattn/go-sqlite3" -) - -const schema = ` -CREATE TABLE IF NOT EXISTS users ( - did TEXT PRIMARY KEY, - handle TEXT NOT NULL, - pds_endpoint TEXT NOT NULL, - last_seen TIMESTAMP NOT NULL, - UNIQUE(handle) -); -CREATE INDEX IF NOT EXISTS idx_users_handle ON users(handle); - -CREATE TABLE IF NOT EXISTS manifests ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - did TEXT NOT NULL, - repository TEXT NOT NULL, - digest TEXT NOT NULL, - hold_endpoint TEXT NOT NULL, - schema_version INTEGER NOT NULL, - media_type TEXT NOT NULL, - config_digest TEXT, - config_size INTEGER, - raw_manifest TEXT NOT NULL, - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, digest), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX IF NOT EXISTS idx_manifests_did_repo ON manifests(did, repository); -CREATE INDEX IF NOT EXISTS idx_manifests_created_at ON manifests(created_at DESC); -CREATE INDEX IF NOT EXISTS idx_manifests_digest ON manifests(digest); - -CREATE TABLE IF NOT EXISTS layers ( - manifest_id INTEGER NOT NULL, - digest TEXT NOT NULL, - size INTEGER NOT NULL, - media_type TEXT NOT NULL, - layer_index INTEGER NOT NULL, - PRIMARY KEY(manifest_id, layer_index), - FOREIGN KEY(manifest_id) REFERENCES manifests(id) ON DELETE CASCADE -); -CREATE INDEX IF NOT EXISTS idx_layers_digest ON layers(digest); - -CREATE TABLE IF NOT EXISTS tags ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - did TEXT NOT NULL, - repository TEXT NOT NULL, - tag TEXT NOT NULL, - digest TEXT NOT NULL, - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, tag), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX IF NOT EXISTS idx_tags_did_repo ON tags(did, repository); - -CREATE TABLE IF NOT EXISTS firehose_cursor ( - id INTEGER PRIMARY KEY CHECK (id = 1), - cursor INTEGER NOT NULL, - updated_at TIMESTAMP NOT NULL -); -` - -func InitDB(path string) (*sql.DB, error) { - db, err := sql.Open("sqlite3", path) - if err != nil { - return nil, err - } - - if _, err := db.Exec(schema); err != nil { - return nil, err - } - - return db, nil -} -``` - -### Data Models - -**pkg/appview/db/models.go:** - -```go -package db - -import "time" - -type User struct { - DID string - Handle string - PDSEndpoint string - LastSeen time.Time -} - -type Manifest struct { - ID int64 - DID string - Repository string - Digest string - HoldEndpoint string - SchemaVersion int - MediaType string - ConfigDigest string - ConfigSize int64 - RawManifest string // JSON - CreatedAt time.Time -} - -type Tag struct { - ID int64 - DID string - Repository string - Tag string - Digest string - CreatedAt time.Time -} - -type Push struct { - Handle string - Repository string - Tag string - Digest string - HoldEndpoint string - CreatedAt time.Time -} - -type Repository struct { - Name string - TagCount int - ManifestCount int - LastPush time.Time - Tags []Tag - Manifests []Manifest -} -``` - -### Query Functions - -**pkg/appview/db/queries.go:** - -```go -package db - -import ( - "database/sql" - "time" -) - -// GetRecentPushes fetches recent pushes with pagination -func GetRecentPushes(db *sql.DB, limit, offset int, userFilter string) ([]Push, int, error) { - query := ` - SELECT u.handle, t.repository, t.tag, t.digest, m.hold_endpoint, t.created_at - FROM tags t - JOIN users u ON t.did = u.did - JOIN manifests m ON t.did = m.did AND t.repository = m.repository AND t.digest = m.digest - ` - - if userFilter != "" { - query += " WHERE u.handle = ? OR u.did = ?" - } - - query += " ORDER BY t.created_at DESC LIMIT ? OFFSET ?" - - var rows *sql.Rows - var err error - - if userFilter != "" { - rows, err = db.Query(query, userFilter, userFilter, limit, offset) - } else { - rows, err = db.Query(query, limit, offset) - } - - if err != nil { - return nil, 0, err - } - defer rows.Close() - - var pushes []Push - for rows.Next() { - var p Push - if err := rows.Scan(&p.Handle, &p.Repository, &p.Tag, &p.Digest, &p.HoldEndpoint, &p.CreatedAt); err != nil { - return nil, 0, err - } - pushes = append(pushes, p) - } - - // Get total count - countQuery := "SELECT COUNT(*) FROM tags t JOIN users u ON t.did = u.did" - if userFilter != "" { - countQuery += " WHERE u.handle = ? OR u.did = ?" - } - - var total int - if userFilter != "" { - db.QueryRow(countQuery, userFilter, userFilter).Scan(&total) - } else { - db.QueryRow(countQuery).Scan(&total) - } - - return pushes, total, nil -} - -// GetUserRepositories fetches all repositories for a user -func GetUserRepositories(db *sql.DB, did string) ([]Repository, error) { - // Get repository summary - rows, err := db.Query(` - SELECT - repository, - COUNT(DISTINCT tag) as tag_count, - COUNT(DISTINCT digest) as manifest_count, - MAX(created_at) as last_push - FROM ( - SELECT repository, tag, digest, created_at FROM tags WHERE did = ? - UNION - SELECT repository, NULL, digest, created_at FROM manifests WHERE did = ? - ) - GROUP BY repository - ORDER BY last_push DESC - `, did, did) - - if err != nil { - return nil, err - } - defer rows.Close() - - var repos []Repository - for rows.Next() { - var r Repository - if err := rows.Scan(&r.Name, &r.TagCount, &r.ManifestCount, &r.LastPush); err != nil { - return nil, err - } - - // Get tags for this repo - tagRows, err := db.Query(` - SELECT tag, digest, created_at - FROM tags - WHERE did = ? AND repository = ? - ORDER BY created_at DESC - `, did, r.Name) - - if err != nil { - return nil, err - } - - for tagRows.Next() { - var t Tag - if err := tagRows.Scan(&t.Tag, &t.Digest, &t.CreatedAt); err != nil { - tagRows.Close() - return nil, err - } - r.Tags = append(r.Tags, t) - } - tagRows.Close() - - // Get manifests for this repo - manifestRows, err := db.Query(` - SELECT id, digest, hold_endpoint, schema_version, media_type, - config_digest, config_size, raw_manifest, created_at - FROM manifests - WHERE did = ? AND repository = ? - ORDER BY created_at DESC - `, did, r.Name) - - if err != nil { - return nil, err - } - - for manifestRows.Next() { - var m Manifest - if err := manifestRows.Scan(&m.ID, &m.Digest, &m.HoldEndpoint, &m.SchemaVersion, - &m.MediaType, &m.ConfigDigest, &m.ConfigSize, &m.RawManifest, &m.CreatedAt); err != nil { - manifestRows.Close() - return nil, err - } - r.Manifests = append(r.Manifests, m) - } - manifestRows.Close() - - repos = append(repos, r) - } - - return repos, nil -} -``` - -## Step 2: Templates Layout - -### Base Layout - -**pkg/appview/templates/layouts/base.html:** - -```html - - - - - - {{ block "title" . }}ATCR{{ end }} - - - {{ block "head" . }}{{ end }} - - - {{ template "nav" . }} - -
- {{ block "content" . }}{{ end }} -
- - - - - - {{ block "scripts" . }}{{ end }} - - -``` - -### Navigation Component - -**pkg/appview/templates/components/nav.html:** - -```html -{{ define "nav" }} - -{{ end }} -``` - -## Step 3: Front Page (Homepage) - -**pkg/appview/templates/pages/home.html:** - -```html -{{ define "title" }}ATCR - Federated Container Registry{{ end }} - -{{ define "content" }} -
-

Recent Pushes

- -
- - -
- -
- -
Loading recent pushes...
-
-
-{{ end }} -``` - -**pkg/appview/templates/partials/push-list.html:** - -```html -{{ range .Pushes }} -
-
- {{ .Handle }} - / - {{ .Repository }} - : - {{ .Tag }} -
- -
- {{ printf "%.12s" .Digest }}... - • - {{ .HoldEndpoint }} - • - -
- -
- docker pull atcr.io/{{ .Handle }}/{{ .Repository }}:{{ .Tag }} - -
- - -
-{{ end }} - -{{ if .HasMore }} - -{{ end }} -``` - -**pkg/appview/handlers/home.go:** - -```go -package handlers - -import ( - "html/template" - "net/http" - "strconv" - "atcr.io/pkg/appview/db" -) - -type HomeHandler struct { - DB *sql.DB - Templates *template.Template -} - -func (h *HomeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Check if this is an HTMX request for the partial - if r.Header.Get("HX-Request") == "true" { - h.servePushList(w, r) - return - } - - // Serve full page - data := struct { - User *db.User - }{ - User: getUserFromContext(r), - } - - h.Templates.ExecuteTemplate(w, "home.html", data) -} - -func (h *HomeHandler) servePushList(w http.ResponseWriter, r *http.Request) { - limit := 50 - offset := 0 - - if o := r.URL.Query().Get("offset"); o != "" { - offset, _ = strconv.Atoi(o) - } - - userFilter := r.URL.Query().Get("user") - - pushes, total, err := db.GetRecentPushes(h.DB, limit, offset, userFilter) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - data := struct { - Pushes []db.Push - HasMore bool - NextOffset int - }{ - Pushes: pushes, - HasMore: offset+limit < total, - NextOffset: offset + limit, - } - - h.Templates.ExecuteTemplate(w, "push-list.html", data) -} -``` - -## Step 4: Settings Page - -**pkg/appview/templates/pages/settings.html:** - -```html -{{ define "title" }}Settings - ATCR{{ end }} - -{{ define "content" }} -
-

Settings

- - -
-

Identity

-
- - {{ .Profile.Handle }} -
-
- - {{ .Profile.DID }} -
-
- - {{ .Profile.PDSEndpoint }} -
-
- - -
-

Default Hold

-

Current: {{ .Profile.DefaultHold }}

- -
- -
- - -
- - - - -
- -
-
- - -
-

OAuth Session

-
- - {{ .Profile.Handle }} -
-
- - -
- Re-authenticate -
-
-{{ end }} - -{{ define "scripts" }} - -{{ end }} -``` - -**pkg/appview/handlers/settings.go:** - -```go -package handlers - -import ( - "database/sql" - "encoding/json" - "html/template" - "net/http" - "atcr.io/pkg/atproto" -) - -type SettingsHandler struct { - Templates *template.Template - ATProtoClient *atproto.Client -} - -func (h *SettingsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - user := getUserFromContext(r) - if user == nil { - http.Redirect(w, r, "/auth/oauth/login?return_to=/ui/settings", http.StatusFound) - return - } - - // Fetch user profile from PDS - profile, err := h.ATProtoClient.GetProfile(user.DID) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - // Fetch user's holds - holds, err := h.ATProtoClient.ListHolds(user.DID) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - data := struct { - Profile *atproto.SailorProfileRecord - Holds []atproto.HoldRecord - SessionExpiry time.Time - }{ - Profile: profile, - Holds: holds, - SessionExpiry: getSessionExpiry(r), - } - - h.Templates.ExecuteTemplate(w, "settings.html", data) -} - -func (h *SettingsHandler) UpdateDefaultHold(w http.ResponseWriter, r *http.Request) { - user := getUserFromContext(r) - if user == nil { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - - holdEndpoint := r.FormValue("hold_endpoint") - if holdEndpoint == "" { - holdEndpoint = r.FormValue("custom_hold") - } - - // Update profile in PDS - err := h.ATProtoClient.UpdateProfile(user.DID, map[string]any{ - "defaultHold": holdEndpoint, - }) - - if err != nil { - w.Write([]byte(`
Failed to update: ` + err.Error() + `
`)) - return - } - - w.Write([]byte(`
✓ Default hold updated successfully!
`)) -} -``` - -## Step 5: Personal Images Page - -**pkg/appview/templates/pages/images.html:** - -```html -{{ define "title" }}Your Images - ATCR{{ end }} - -{{ define "content" }} -
-

Your Images

- - {{ if .Repositories }} - {{ range .Repositories }} -
-
-

{{ .Name }}

-
- {{ .TagCount }} tags - • - {{ .ManifestCount }} manifests - • - -
- -
- - -
- {{ end }} - {{ else }} -
-

No images yet. Push your first image:

- docker push atcr.io/{{ .User.Handle }}/myapp:latest -
- {{ end }} -
-{{ end }} - -{{ define "scripts" }} - -{{ end }} -``` - -**pkg/appview/handlers/images.go:** - -```go -package handlers - -import ( - "database/sql" - "html/template" - "net/http" - "atcr.io/pkg/appview/db" - "atcr.io/pkg/atproto" -) - -type ImagesHandler struct { - DB *sql.DB - Templates *template.Template - ATProtoClient *atproto.Client -} - -func (h *ImagesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - user := getUserFromContext(r) - if user == nil { - http.Redirect(w, r, "/auth/oauth/login?return_to=/ui/images", http.StatusFound) - return - } - - // Fetch repositories from PDS (user's own data) - repos, err := h.ATProtoClient.ListRepositories(user.DID) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - data := struct { - User *db.User - Repositories []db.Repository - }{ - User: user, - Repositories: repos, - } - - h.Templates.ExecuteTemplate(w, "images.html", data) -} - -func (h *ImagesHandler) DeleteTag(w http.ResponseWriter, r *http.Request) { - user := getUserFromContext(r) - if user == nil { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - - // Extract repo and tag from URL - vars := mux.Vars(r) - repo := vars["repository"] - tag := vars["tag"] - - // Delete tag record from PDS - err := h.ATProtoClient.DeleteTag(user.DID, repo, tag) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - // Return empty response (HTMX will swap out the element) - w.WriteHeader(http.StatusOK) -} - -func (h *ImagesHandler) DeleteManifest(w http.ResponseWriter, r *http.Request) { - user := getUserFromContext(r) - if user == nil { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - - vars := mux.Vars(r) - repo := vars["repository"] - digest := vars["digest"] - - // Check if manifest is tagged - tagged, err := h.ATProtoClient.IsManifestTagged(user.DID, repo, digest) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - if tagged { - http.Error(w, "Cannot delete tagged manifest", http.StatusBadRequest) - return - } - - // Delete manifest from PDS - err = h.ATProtoClient.DeleteManifest(user.DID, repo, digest) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - w.WriteHeader(http.StatusOK) -} -``` - -## Step 6: Modals & Partials - -**pkg/appview/templates/components/modal.html:** - -```html -{{ define "manifest-modal" }} -
-
- - -

Manifest Details

- -
-
- Digest: - {{ .Digest }} -
-
- Media Type: - {{ .MediaType }} -
-
- Size: - {{ .Size | humanizeBytes }} -
-
- Architecture: - {{ .Architecture }}/{{ .OS }} -
-
- Created: - -
-
- ATProto Record: - - View on PDS - -
-
- -

Layers

-
- {{ range .Layers }} -
- {{ .Digest }} - {{ .Size | humanizeBytes }} - {{ .MediaType }} -
- {{ end }} -
- -

Raw Manifest

- 
{{ .RawManifest }}
-
-
-{{ end }} -``` - -**pkg/appview/templates/partials/edit-tag-modal.html:** - -```html -
-
- - -

Edit Tag: {{ .Tag }}

- -
- -
- - -
- - - -
-
-
-``` - -## Step 7: Authentication & Session - -**pkg/appview/session/session.go:** - -```go -package session - -import ( - "crypto/rand" - "encoding/base64" - "net/http" - "sync" - "time" -) - -type Session struct { - ID string - DID string - Handle string - ExpiresAt time.Time -} - -type Store struct { - mu sync.RWMutex - sessions map[string]*Session -} - -func NewStore() *Store { - return &Store{ - sessions: make(map[string]*Session), - } -} - -func (s *Store) Create(did, handle string, duration time.Duration) (*Session, error) { - s.mu.Lock() - defer s.mu.Unlock() - - // Generate random session ID - b := make([]byte, 32) - if _, err := rand.Read(b); err != nil { - return nil, err - } - - sess := &Session{ - ID: base64.URLEncoding.EncodeToString(b), - DID: did, - Handle: handle, - ExpiresAt: time.Now().Add(duration), - } - - s.sessions[sess.ID] = sess - return sess, nil -} - -func (s *Store) Get(id string) (*Session, bool) { - s.mu.RLock() - defer s.mu.RUnlock() - - sess, ok := s.sessions[id] - if !ok || time.Now().After(sess.ExpiresAt) { - return nil, false - } - - return sess, true -} - -func (s *Store) Delete(id string) { - s.mu.Lock() - defer s.mu.Unlock() - - delete(s.sessions, id) -} - -func (s *Store) Cleanup() { - s.mu.Lock() - defer s.mu.Unlock() - - now := time.Now() - for id, sess := range s.sessions { - if now.After(sess.ExpiresAt) { - delete(s.sessions, id) - } - } -} - -// SetCookie sets the session cookie -func SetCookie(w http.ResponseWriter, sessionID string, maxAge int) { - http.SetCookie(w, &http.Cookie{ - Name: "atcr_session", - Value: sessionID, - Path: "/", - MaxAge: maxAge, - HttpOnly: true, - Secure: true, - SameSite: http.SameSiteLaxMode, - }) -} - -// GetSessionID gets session ID from cookie -func GetSessionID(r *http.Request) (string, bool) { - cookie, err := r.Cookie("atcr_session") - if err != nil { - return "", false - } - return cookie.Value, true -} -``` - -**pkg/appview/middleware/auth.go:** - -```go -package middleware - -import ( - "context" - "net/http" - "atcr.io/pkg/appview/session" - "atcr.io/pkg/appview/db" -) - -type contextKey string - -const userKey contextKey = "user" - -func RequireAuth(store *session.Store) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - sessionID, ok := session.GetSessionID(r) - if !ok { - http.Redirect(w, r, "/auth/oauth/login?return_to="+r.URL.Path, http.StatusFound) - return - } - - sess, ok := store.Get(sessionID) - if !ok { - http.Redirect(w, r, "/auth/oauth/login?return_to="+r.URL.Path, http.StatusFound) - return - } - - user := &db.User{ - DID: sess.DID, - Handle: sess.Handle, - } - - ctx := context.WithValue(r.Context(), userKey, user) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } -} - -func GetUser(r *http.Request) *db.User { - user, ok := r.Context().Value(userKey).(*db.User) - if !ok { - return nil - } - return user -} -``` - -## Step 8: Main Integration - -**cmd/appview/main.go (additions):** - -```go -package main - -import ( - "log" - "net/http" - "time" - - "github.com/gorilla/mux" - "atcr.io/pkg/appview" - "atcr.io/pkg/appview/handlers" - "atcr.io/pkg/appview/db" - "atcr.io/pkg/appview/session" - "atcr.io/pkg/appview/middleware" -) - -func main() { - // Initialize database - database, err := db.InitDB("/var/lib/atcr/ui.db") - if err != nil { - log.Fatal(err) - } - - // Initialize session store - sessionStore := session.NewStore() - - // Start cleanup goroutine - go func() { - for { - time.Sleep(5 * time.Minute) - sessionStore.Cleanup() - } - }() - - // Load embedded templates - tmpl, err := appview.Templates() - if err != nil { - log.Fatal(err) - } - - // Setup router - r := mux.NewRouter() - - // Static files (embedded) - r.PathPrefix("/static/").Handler(http.StripPrefix("/static/", appview.StaticHandler())) - - // UI routes (public) - r.Handle("/ui/", &handlers.HomeHandler{ - DB: database, - Templates: tmpl, - }) - - // UI routes (authenticated) - authRouter := r.PathPrefix("/ui").Subrouter() - authRouter.Use(middleware.RequireAuth(sessionStore)) - - authRouter.Handle("/images", &handlers.ImagesHandler{ - DB: database, - Templates: tmpl, - }) - - authRouter.Handle("/settings", &handlers.SettingsHandler{ - Templates: tmpl, - }) - - // API routes - authRouter.HandleFunc("/api/images/{repository}/tags/{tag}", - handlers.DeleteTag).Methods("DELETE") - authRouter.HandleFunc("/api/images/{repository}/manifests/{digest}", - handlers.DeleteManifest).Methods("DELETE") - - // ... rest of your existing routes - - log.Println("Server starting on :5000") - http.ListenAndServe(":5000", r) -} -``` - -## Step 9: Styling (Basic CSS) - -**pkg/appview/static/css/style.css:** - -```css -:root { - --primary: #0066cc; - --bg: #ffffff; - --fg: #1a1a1a; - --border: #e0e0e0; - --code-bg: #f5f5f5; -} - -* { - margin: 0; - padding: 0; - box-sizing: border-box; -} - -body { - font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif; - background: var(--bg); - color: var(--fg); - line-height: 1.6; -} - -.container { - max-width: 1200px; - margin: 0 auto; - padding: 20px; -} - -/* Navigation */ -.navbar { - background: var(--fg); - color: white; - padding: 1rem 2rem; - display: flex; - justify-content: space-between; - align-items: center; -} - -.nav-brand a { - color: white; - text-decoration: none; - font-size: 1.5rem; - font-weight: bold; -} - -.nav-links { - display: flex; - gap: 1rem; - align-items: center; -} - -.nav-links a { - color: white; - text-decoration: none; -} - -/* Push Cards */ -.push-card { - border: 1px solid var(--border); - border-radius: 8px; - padding: 1rem; - margin-bottom: 1rem; - background: white; -} - -.push-header { - font-size: 1.1rem; - margin-bottom: 0.5rem; -} - -.push-user { - color: var(--primary); - text-decoration: none; -} - -.push-command { - display: flex; - gap: 0.5rem; - align-items: center; - margin-top: 0.5rem; - padding: 0.5rem; - background: var(--code-bg); - border-radius: 4px; -} - -.pull-command { - flex: 1; - font-family: 'Monaco', 'Courier New', monospace; - font-size: 0.9rem; -} - -.copy-btn { - padding: 0.25rem 0.5rem; - background: var(--primary); - color: white; - border: none; - border-radius: 4px; - cursor: pointer; -} - -/* Repository Cards */ -.repository-card { - border: 1px solid var(--border); - border-radius: 8px; - margin-bottom: 1rem; - background: white; -} - -.repo-header { - padding: 1rem; - cursor: pointer; - display: flex; - justify-content: space-between; - align-items: center; - background: #f9f9f9; - border-radius: 8px 8px 0 0; -} - -.repo-header:hover { - background: #f0f0f0; -} - -.repo-details { - padding: 1rem; -} - -.tag-row, .manifest-row { - display: flex; - gap: 1rem; - align-items: center; - padding: 0.5rem; - border-bottom: 1px solid var(--border); -} - -.tag-row:last-child, .manifest-row:last-child { - border-bottom: none; -} - -/* Modal */ -.modal-overlay { - position: fixed; - top: 0; - left: 0; - right: 0; - bottom: 0; - background: rgba(0, 0, 0, 0.5); - display: flex; - justify-content: center; - align-items: center; - z-index: 1000; -} - -.modal-content { - background: white; - padding: 2rem; - border-radius: 8px; - max-width: 800px; - max-height: 80vh; - overflow-y: auto; - position: relative; -} - -.modal-close { - position: absolute; - top: 1rem; - right: 1rem; - background: none; - border: none; - font-size: 1.5rem; - cursor: pointer; -} - -.manifest-json { - background: var(--code-bg); - padding: 1rem; - border-radius: 4px; - overflow-x: auto; - font-family: 'Monaco', 'Courier New', monospace; - font-size: 0.85rem; -} - -/* Buttons */ -button, .btn { - padding: 0.5rem 1rem; - background: var(--primary); - color: white; - border: none; - border-radius: 4px; - cursor: pointer; - text-decoration: none; - display: inline-block; -} - -button:hover, .btn:hover { - opacity: 0.9; -} - -.delete-btn { - background: #dc3545; -} - -/* Loading state */ -.loading { - text-align: center; - padding: 2rem; - color: #666; -} - -/* Forms */ -.form-group { - margin-bottom: 1rem; -} - -.form-group label { - display: block; - margin-bottom: 0.5rem; - font-weight: 500; -} - -.form-group input, -.form-group select { - width: 100%; - padding: 0.5rem; - border: 1px solid var(--border); - border-radius: 4px; - font-size: 1rem; -} -``` - -## Step 10: Helper Functions - -**pkg/appview/static/js/app.js:** - -```javascript -// Copy to clipboard -function copyToClipboard(text) { - navigator.clipboard.writeText(text).then(() => { - // Show success feedback - const btn = event.target; - const originalText = btn.textContent; - btn.textContent = '✓ Copied!'; - setTimeout(() => { - btn.textContent = originalText; - }, 2000); - }); -} - -// Time ago helper (for client-side rendering) -function timeAgo(date) { - const seconds = Math.floor((new Date() - new Date(date)) / 1000); - - const intervals = { - year: 31536000, - month: 2592000, - week: 604800, - day: 86400, - hour: 3600, - minute: 60, - second: 1 - }; - - for (const [name, secondsInInterval] of Object.entries(intervals)) { - const interval = Math.floor(seconds / secondsInInterval); - if (interval >= 1) { - return interval === 1 ? `1 ${name} ago` : `${interval} ${name}s ago`; - } - } - - return 'just now'; -} - -// Update timestamps on page load -document.addEventListener('DOMContentLoaded', () => { - document.querySelectorAll('time[datetime]').forEach(el => { - const date = el.getAttribute('datetime'); - el.textContent = timeAgo(date); - }); -}); -``` - -**Template helper functions (in Go):** - -```go -// Add to your template loading -funcMap := template.FuncMap{ - "timeAgo": func(t time.Time) string { - duration := time.Since(t) - - if duration < time.Minute { - return "just now" - } else if duration < time.Hour { - mins := int(duration.Minutes()) - if mins == 1 { - return "1 minute ago" - } - return fmt.Sprintf("%d minutes ago", mins) - } else if duration < 24*time.Hour { - hours := int(duration.Hours()) - if hours == 1 { - return "1 hour ago" - } - return fmt.Sprintf("%d hours ago", hours) - } else { - days := int(duration.Hours() / 24) - if days == 1 { - return "1 day ago" - } - return fmt.Sprintf("%d days ago", days) - } - }, - - "humanizeBytes": func(bytes int64) string { - const unit = 1024 - if bytes < unit { - return fmt.Sprintf("%d B", bytes) - } - div, exp := int64(unit), 0 - for n := bytes / unit; n >= unit; n /= unit { - div *= unit - exp++ - } - return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp]) - }, -} - -tmpl := template.New("").Funcs(funcMap) -tmpl = template.Must(tmpl.ParseGlob("web/templates/**/*.html")) -``` - -## Implementation Checklist - -### Phase 1: Foundation -- [ ] Set up project structure -- [ ] Initialize SQLite database with schema -- [ ] Create data models and query functions -- [ ] Write database tests - -### Phase 2: Templates -- [ ] Create base layout template -- [ ] Create navigation component -- [ ] Create home page template -- [ ] Create settings page template -- [ ] Create images page template -- [ ] Create modal templates - -### Phase 3: Handlers -- [ ] Implement home handler (firehose display) -- [ ] Implement settings handler (profile + holds) -- [ ] Implement images handler (repository list) -- [ ] Implement API endpoints (delete tag, delete manifest) -- [ ] Add HTMX partial responses - -### Phase 4: Authentication -- [ ] Implement session store -- [ ] Create auth middleware -- [ ] Wire up OAuth login (reuse existing) -- [ ] Add logout functionality -- [ ] Test auth flow - -### Phase 5: Firehose Worker -- [ ] Implement Jetstream client -- [ ] Create firehose worker -- [ ] Add event handlers (manifest, tag) -- [ ] Test with real firehose -- [ ] Add cursor persistence - -### Phase 6: Polish -- [ ] Add CSS styling -- [ ] Implement copy-to-clipboard -- [ ] Add loading states -- [ ] Error handling and user feedback -- [ ] Responsive design -- [ ] CSRF protection - -### Phase 7: Testing -- [ ] Unit tests for handlers -- [ ] Database query tests -- [ ] Integration tests (full flow) -- [ ] Manual testing with real data - -## Performance Optimizations - -### HTMX Optimizations -1. **Prefetching:** Add `hx-trigger="mouseenter"` to links for hover prefetch -2. **Caching:** Use `hx-cache="true"` for cacheable content -3. **Optimistic updates:** Remove elements immediately, rollback on error -4. **Debouncing:** Add `delay:500ms` to search inputs - -### Database Optimizations -1. **Indexes:** Already defined in schema (did, repo, created_at, digest) -2. **Connection pooling:** Use `db.SetMaxOpenConns(25)` -3. **Prepared statements:** Cache frequently used queries -4. **Batch inserts:** For firehose events, batch into transactions - -### Template Optimizations -1. **Pre-parse:** Parse templates once at startup, not per request -2. **Caching:** Cache rendered partials for static content -3. **Minification:** Minify HTML/CSS/JS in production - -## Security Checklist - -- [ ] Session cookies: Secure, HttpOnly, SameSite=Lax -- [ ] CSRF tokens for mutations (POST/DELETE) -- [ ] Input validation (sanitize search, filters) -- [ ] Rate limiting on API endpoints -- [ ] SQL injection protection (parameterized queries) -- [ ] Authorization checks (user owns resource) -- [ ] XSS protection (escape template output) - -## Deployment - -### Development -```bash -# Run migrations -go run cmd/appview/main.go migrate - -# Start server -go run cmd/appview/main.go serve -``` - -### Production -```bash -# Build binary -go build -o bin/atcr-appview ./cmd/appview - -# Run with config -./bin/atcr-appview serve config/production.yml -``` - -### Environment Variables -```bash -UI_ENABLED=true -UI_DATABASE_PATH=/var/lib/atcr/ui.db -UI_FIREHOSE_ENDPOINT=wss://jetstream.atproto.tools/subscribe -UI_SESSION_DURATION=24h -``` - -## Next Steps After V1 - -1. **Add search:** Implement full-text search on SQLite -2. **Public profiles:** `/ui/@alice` shows public view -3. **Manifest diff:** Compare manifest versions -4. **Export data:** Download all your images as JSON -5. **Webhook notifications:** Alert on new pushes -6. **CLI integration:** `atcr ui open` to launch browser - ---- - -## Key Benefits of This Approach - -### Single Binary Deployment -- All templates and static files embedded with `//go:embed` -- No need to ship separate `web/` directory -- Single `atcr-appview` binary contains everything -- Easy deployment: just copy one file - -### Package Structure -- `pkg/appview` makes sense semantically (it's the AppView, not just UI) -- Contains both backend (db, firehose) and frontend (templates, handlers) -- Clear separation from core OCI registry logic -- Easy to test and develop independently - -### Embedded Assets -```go -// pkg/appview/appview.go -//go:embed templates/*.html templates/**/*.html -var templatesFS embed.FS - -//go:embed static/* -var staticFS embed.FS -``` - -**Build:** -```bash -go build -o bin/atcr-appview ./cmd/appview -``` - -**Deploy:** -```bash -scp bin/atcr-appview server:/usr/local/bin/ -# Done! No webpack, no node_modules, no separate assets folder -``` - -### Development Workflow -1. Edit templates in `pkg/appview/templates/` -2. Edit CSS/JS in `pkg/appview/static/` -3. Run `go build` - assets auto-embedded -4. No build tools, no npm, just Go - ---- - -This guide provides a complete implementation path for ATCR AppView UI using html/template + HTMX with embedded assets. Start with Phase 1 (embed setup + database) and work your way through each phase sequentially. diff --git a/docs/APPVIEW-UI-V1.md b/docs/APPVIEW-UI-V1.md deleted file mode 100644 index 48d0319..0000000 --- a/docs/APPVIEW-UI-V1.md +++ /dev/null @@ -1,631 +0,0 @@ -# ATCR AppView UI - Version 1 Specification - -## Overview - -The ATCR AppView UI provides a web interface for discovering, managing, and configuring container images in the ATCR registry. Version 1 focuses on three core pages that leverage existing functionality: - -1. **Front Page** - Distributed image discovery via firehose -2. **Settings Page** - Profile and hold configuration -3. **Personal Page** - Manage your images and tags - -## Architecture - -### Tech Stack - -- **Backend:** Go (existing AppView codebase) -- **Frontend:** TBD (Go templates/Templ or separate SPA) -- **Database:** SQLite (firehose data cache) -- **Styling:** TBD (plain CSS, Tailwind, etc.) -- **Authentication:** ATProto OAuth (DPoP handled by indigo library) - -### Components - -``` -┌─────────────────────────────────────────────────────────────┐ -│ Web UI (Browser) │ -└─────────────────────────────────────────────────────────────┘ - │ - ▼ -┌─────────────────────────────────────────────────────────────┐ -│ AppView HTTP Server │ -│ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │ -│ │ UI Endpoints │ │ OCI API │ │ OAuth Server │ │ -│ │ /ui/* │ │ /v2/* │ │ /auth/* │ │ -│ └──────────────┘ └──────────────┘ └──────────────┘ │ -└─────────────────────────────────────────────────────────────┘ - │ - ┌─────────┴─────────┐ - ▼ ▼ - ┌──────────────────┐ ┌──────────────────┐ - │ SQLite Database │ │ ATProto Client │ - │ (Firehose cache) │ │ (PDS operations) │ - └──────────────────┘ └──────────────────┘ - ▲ - ┌──────────────────┐ │ - │ Firehose Worker │───────────┘ - │ (Background) │ - └──────────────────┘ - ▲ - │ - ┌──────────────────┐ - │ ATProto Firehose │ - │ (Jetstream/Relay)│ - └──────────────────┘ -``` - -## Database Schema - -SQLite database for caching firehose data and enabling fast queries. - -### Tables - -**users** -```sql -CREATE TABLE users ( - did TEXT PRIMARY KEY, - handle TEXT NOT NULL, - pds_endpoint TEXT NOT NULL, - last_seen TIMESTAMP NOT NULL, - UNIQUE(handle) -); -CREATE INDEX idx_users_handle ON users(handle); -``` - -**manifests** -```sql -CREATE TABLE manifests ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - did TEXT NOT NULL, - repository TEXT NOT NULL, - digest TEXT NOT NULL, - hold_endpoint TEXT NOT NULL, - schema_version INTEGER NOT NULL, - media_type TEXT NOT NULL, - config_digest TEXT, - config_size INTEGER, - raw_manifest TEXT NOT NULL, -- JSON blob - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, digest), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX idx_manifests_did_repo ON manifests(did, repository); -CREATE INDEX idx_manifests_created_at ON manifests(created_at DESC); -CREATE INDEX idx_manifests_digest ON manifests(digest); -``` - -**layers** -```sql -CREATE TABLE layers ( - manifest_id INTEGER NOT NULL, - digest TEXT NOT NULL, - size INTEGER NOT NULL, - media_type TEXT NOT NULL, - layer_index INTEGER NOT NULL, - PRIMARY KEY(manifest_id, layer_index), - FOREIGN KEY(manifest_id) REFERENCES manifests(id) ON DELETE CASCADE -); -CREATE INDEX idx_layers_digest ON layers(digest); -``` - -**tags** -```sql -CREATE TABLE tags ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - did TEXT NOT NULL, - repository TEXT NOT NULL, - tag TEXT NOT NULL, - digest TEXT NOT NULL, - created_at TIMESTAMP NOT NULL, - UNIQUE(did, repository, tag), - FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE -); -CREATE INDEX idx_tags_did_repo ON tags(did, repository); -``` - -**firehose_cursor** -```sql -CREATE TABLE firehose_cursor ( - id INTEGER PRIMARY KEY CHECK (id = 1), - cursor INTEGER NOT NULL, - updated_at TIMESTAMP NOT NULL -); -``` - -## Firehose Worker - -Background goroutine that subscribes to ATProto firehose and populates the database. - -### Implementation - -```go -// pkg/ui/firehose/worker.go - -type Worker struct { - db *sql.DB - jetstream *JetstreamClient - resolver *atproto.Resolver - stopCh chan struct{} -} - -func (w *Worker) Start() error { - // Load cursor from database - cursor := w.loadCursor() - - // Subscribe to firehose - events := w.jetstream.Subscribe(cursor, []string{ - "io.atcr.manifest", - "io.atcr.tag", - }) - - for { - select { - case event := <-events: - w.handleEvent(event) - case <-w.stopCh: - return nil - } - } -} - -func (w *Worker) handleEvent(event FirehoseEvent) error { - switch event.Collection { - case "io.atcr.manifest": - return w.handleManifest(event) - case "io.atcr.tag": - return w.handleTag(event) - } - return nil -} -``` - -### Event Handling - -**Manifest create:** -- Resolve DID → handle, PDS endpoint -- Insert/update user record -- Parse manifest JSON -- Insert manifest record -- Insert layer records - -**Tag create/update:** -- Insert/update tag record -- Link to existing manifest - -**Record deletion:** -- Delete from database (cascade handles related records) - -### Firehose Connection - -Use Jetstream (bluesky-social/jetstream) or connect directly to relay: -- **Jetstream:** Websocket to `wss://jetstream.atproto.tools/subscribe` -- **Relay:** Websocket to relay (e.g., `wss://bsky.network/xrpc/com.atproto.sync.subscribeRepos`) - -Jetstream is simpler and filters events server-side. - -## Page Specifications - -### 1. Front Page - Distributed Discovery - -**URL:** `/ui/` or `/ui/explore` - -**Purpose:** Discover recently pushed images across all ATCR users. - -**Layout:** -``` -┌─────────────────────────────────────────────────────────────┐ -│ ATCR [Search] [@handle] [Login] │ -├─────────────────────────────────────────────────────────────┤ -│ Recent Pushes [Filter ▼]│ -│ │ -│ ┌───────────────────────────────────────────────────────┐ │ -│ │ alice.bsky.social/nginx:latest │ │ -│ │ sha256:abc123... • hold1.alice.com • 2 hours ago │ │ -│ │ [docker pull atcr.io/alice.bsky.social/nginx:latest] │ │ -│ └───────────────────────────────────────────────────────┘ │ -│ │ -│ ┌───────────────────────────────────────────────────────┐ │ -│ │ bob.dev/myapp:v1.2.3 │ │ -│ │ sha256:def456... • atcr-storage.fly.dev • 5 hours ago │ │ -│ │ [docker pull atcr.io/bob.dev/myapp:v1.2.3] │ │ -│ └───────────────────────────────────────────────────────┘ │ -│ │ -│ [Load more...] │ -└─────────────────────────────────────────────────────────────┘ -``` - -**Features:** -- List of recent pushes (manifests + tags) -- Show: handle, repository, tag, digest (truncated), timestamp, hold endpoint -- Copy-paste pull command with click-to-copy -- Filter by user (click handle to filter) -- Search by repository name or tag -- Click manifest to view details (modal or dedicated page) -- Pagination (50 items per page) - -**API Endpoint:** -``` -GET /ui/api/recent-pushes -Query params: - - limit (default: 50) - - offset (default: 0) - - user (optional: filter by DID or handle) - - repository (optional: filter by repo name) - -Response: -{ - "pushes": [ - { - "did": "did:plc:alice123", - "handle": "alice.bsky.social", - "repository": "nginx", - "tag": "latest", - "digest": "sha256:abc123...", - "hold_endpoint": "https://hold1.alice.com", - "created_at": "2025-10-05T12:34:56Z", - "pull_command": "docker pull atcr.io/alice.bsky.social/nginx:latest" - } - ], - "total": 1234, - "offset": 0, - "limit": 50 -} -``` - -**Manifest Details Modal:** -- Full manifest JSON (syntax highlighted) -- Layer list with digests and sizes -- Link to ATProto record (at://did/io.atcr.manifest/rkey) -- Architecture, OS, labels -- Creation timestamp - -### 2. Settings Page - -**URL:** `/ui/settings` - -**Auth:** Requires login (OAuth) - -**Purpose:** Configure profile and hold preferences. - -**Layout:** -``` -┌─────────────────────────────────────────────────────────────┐ -│ ATCR [@alice] [⚙️] │ -├─────────────────────────────────────────────────────────────┤ -│ Settings │ -│ │ -│ ┌─ Identity ───────────────────────────────────────────┐ │ -│ │ Handle: alice.bsky.social │ │ -│ │ DID: did:plc:alice123abc (read-only) │ │ -│ │ PDS: https://bsky.social (read-only) │ │ -│ └───────────────────────────────────────────────────────┘ │ -│ │ -│ ┌─ Default Hold ──────────────────────────────────────┐ │ -│ │ Current: https://hold1.alice.com │ │ -│ │ │ │ -│ │ [Dropdown: Select from your holds ▼] │ │ -│ │ • https://hold1.alice.com (Your BYOS) │ │ -│ │ • https://storage.atcr.io (AppView default) │ │ -│ │ • [Custom URL...] │ │ -│ │ │ │ -│ │ Custom hold URL: [_____________________] │ │ -│ │ │ │ -│ │ [Save] │ │ -│ └───────────────────────────────────────────────────────┘ │ -│ │ -│ ┌─ OAuth Session ─────────────────────────────────────┐ │ -│ │ Logged in as: alice.bsky.social │ │ -│ │ Session expires: 2025-10-06 14:23:00 UTC │ │ -│ │ [Re-authenticate] │ │ -│ └───────────────────────────────────────────────────────┘ │ -└─────────────────────────────────────────────────────────────┘ -``` - -**Features:** -- Display current identity (handle, DID, PDS) -- Default hold configuration: - - Dropdown showing user's `io.atcr.hold` records (query from PDS) - - Option to select AppView's default storage endpoint - - Manual entry for custom hold URL - - "Save" button updates `io.atcr.sailor.profile.defaultHold` -- OAuth session status -- Re-authenticate button (redirects to OAuth flow) - -**API Endpoints:** - -``` -GET /ui/api/profile -Auth: Required (session cookie) -Response: -{ - "did": "did:plc:alice123", - "handle": "alice.bsky.social", - "pds_endpoint": "https://bsky.social", - "default_hold": "https://hold1.alice.com", - "holds": [ - { - "endpoint": "https://hold1.alice.com", - "name": "My BYOS Storage", - "public": false - } - ], - "session_expires_at": "2025-10-06T14:23:00Z" -} - -POST /ui/api/profile/default-hold -Auth: Required -Body: -{ - "hold_endpoint": "https://hold1.alice.com" -} -Response: -{ - "success": true -} -``` - -### 3. Personal Page - Your Images - -**URL:** `/ui/images` or `/ui/@{handle}` - -**Auth:** Requires login (OAuth) - -**Purpose:** Manage your container images and tags. - -**Layout:** -``` -┌─────────────────────────────────────────────────────────────┐ -│ ATCR [@alice] [⚙️] │ -├─────────────────────────────────────────────────────────────┤ -│ Your Images │ -│ │ -│ ┌─ nginx ──────────────────────────────────────────────┐ │ -│ │ 3 tags • 5 manifests • Last push: 2 hours ago │ │ -│ │ │ │ -│ │ Tags: │ │ -│ │ ┌────────────────────────────────────────────────┐ │ │ -│ │ │ latest → sha256:abc123... (2 hours ago) [✏️][🗑️]│ │ │ -│ │ │ v1.25 → sha256:def456... (1 day ago) [✏️][🗑️]│ │ │ -│ │ │ alpine → sha256:ghi789... (3 days ago) [✏️][🗑️]│ │ │ -│ │ └────────────────────────────────────────────────┘ │ │ -│ │ │ │ -│ │ Manifests: │ │ -│ │ ┌────────────────────────────────────────────────┐ │ │ -│ │ │ sha256:abc123... • 45MB • hold1.alice.com │ │ │ -│ │ │ linux/amd64 • 5 layers • [View] [Delete] │ │ │ -│ │ │ sha256:def456... • 42MB • hold1.alice.com │ │ │ -│ │ │ linux/amd64 • 5 layers • [View] [Delete] │ │ │ -│ │ └────────────────────────────────────────────────┘ │ │ -│ └───────────────────────────────────────────────────────┘ │ -│ │ -│ ┌─ myapp ──────────────────────────────────────────────┐ │ -│ │ 2 tags • 2 manifests • Last push: 1 day ago │ │ -│ │ [Expand ▼] │ │ -│ └───────────────────────────────────────────────────────┘ │ -└─────────────────────────────────────────────────────────────┘ -``` - -**Features:** - -**Repository List:** -- Group manifests by repository name -- Show: tag count, manifest count, last push time -- Collapsible/expandable repository cards - -**Repository Details (Expanded):** -- **Tags:** Table showing tag → manifest digest → timestamp - - Edit tag: Modal to re-point tag to different manifest digest - - Delete tag: Confirm dialog, removes `io.atcr.tag` record from PDS -- **Manifests:** List of all manifests in repository - - Show: digest (truncated), size, hold endpoint, architecture, layer count - - View: Open manifest details modal (same as front page) - - Delete: Confirm dialog with warning if manifest is tagged - -**Actions:** -- Copy pull command for each tag -- Edit tag (re-point to different digest) -- Delete tag -- Delete manifest (with validation) - -**API Endpoints:** - -``` -GET /ui/api/images -Auth: Required -Response: -{ - "repositories": [ - { - "name": "nginx", - "tag_count": 3, - "manifest_count": 5, - "last_push": "2025-10-05T10:23:45Z", - "tags": [ - { - "tag": "latest", - "digest": "sha256:abc123...", - "created_at": "2025-10-05T10:23:45Z" - } - ], - "manifests": [ - { - "digest": "sha256:abc123...", - "size": 47185920, - "hold_endpoint": "https://hold1.alice.com", - "architecture": "amd64", - "os": "linux", - "layer_count": 5, - "created_at": "2025-10-05T10:23:45Z", - "tagged": true - } - ] - } - ] -} - -PUT /ui/api/images/{repository}/tags/{tag} -Auth: Required -Body: -{ - "digest": "sha256:new-digest..." -} -Response: -{ - "success": true -} - -DELETE /ui/api/images/{repository}/tags/{tag} -Auth: Required -Response: -{ - "success": true -} - -DELETE /ui/api/images/{repository}/manifests/{digest} -Auth: Required -Response: -{ - "success": true -} -``` - -## Authentication - -### OAuth Login Flow - -Reuse existing OAuth implementation from credential helper and AppView. - -**Login Endpoint:** `/auth/oauth/login` - -**Flow:** -1. User clicks "Login" on UI -2. Redirects to `/auth/oauth/login?return_to=/ui/images` -3. User enters handle (e.g., "alice.bsky.social") -4. Server resolves handle → DID → PDS → OAuth server -5. Server initiates ATProto OAuth flow with PAR (DPoP handled by indigo library) -6. User redirected to PDS for authorization -7. OAuth callback to `/auth/oauth/callback` -8. Server exchanges code for token, validates with PDS -9. Server creates session cookie (secure, httpOnly, SameSite) -10. Redirects to `return_to` URL or default `/ui/images` - -**Session Management:** -- Session cookie: `atcr_session` (JWT or opaque token) -- Session storage: In-memory map or SQLite table -- Session duration: 24 hours (or match OAuth token expiry) -- Refresh: Auto-refresh OAuth token when needed - -**Middleware:** -```go -// pkg/ui/middleware/auth.go - -func RequireAuth(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session := getSession(r) - if session == nil { - http.Redirect(w, r, "/auth/oauth/login?return_to="+r.URL.Path, http.StatusFound) - return - } - - // Add session info to context - ctx := context.WithValue(r.Context(), "session", session) - next.ServeHTTP(w, r.WithContext(ctx)) - }) -} -``` - -## Implementation Roadmap - -### Phase 1: Database & Firehose -1. Define SQLite schema -2. Implement database layer (pkg/ui/db/) -3. Implement firehose worker (pkg/ui/firehose/) -4. Test worker with real firehose - -### Phase 2: API Endpoints -1. Implement `/ui/api/recent-pushes` (front page data) -2. Implement `/ui/api/profile` (settings page data) -3. Implement `/ui/api/images` (personal page data) -4. Implement tag/manifest mutation endpoints - -### Phase 3: Authentication -1. Implement OAuth login endpoint -2. Implement session management -3. Add auth middleware -4. Test login flow - -### Phase 4: Frontend -1. Choose framework (templates vs SPA) -2. Implement front page -3. Implement settings page -4. Implement personal page -5. Add styling - -### Phase 5: Polish -1. Error handling -2. Loading states -3. Responsive design -4. Testing - -## Open Questions - -1. **Framework choice:** Go templates (Templ?), HTMX, or SPA (React/Vue)? -2. **Styling:** Tailwind, plain CSS, or component library? -3. **Manifest details:** Modal vs dedicated page? -4. **Search:** Full-text search on repository/tag names? Requires FTS in SQLite. -5. **Real-time updates:** WebSocket for firehose events, or polling? -6. **Image size calculation:** Sum of layer sizes, or read from manifest? -7. **Public profiles:** Should `/ui/@alice` show public view of alice's images? -8. **Firehose resilience:** Reconnect logic, backfill on downtime? - -## Dependencies - -New Go packages needed: -- `github.com/mattn/go-sqlite3` - SQLite driver -- `github.com/bluesky-social/jetstream` - Firehose client (or direct websocket) -- Session management library (or custom implementation) -- Frontend framework (TBD) - -## Configuration - -Add to `config/config.yml`: - -```yaml -ui: - enabled: true - database_path: /var/lib/atcr/ui.db - firehose: - enabled: true - endpoint: wss://jetstream.atproto.tools/subscribe - collections: - - io.atcr.manifest - - io.atcr.tag - session: - duration: 24h - cookie_name: atcr_session - cookie_secure: true -``` - -## Security Considerations - -1. **Session cookies:** Secure, HttpOnly, SameSite=Lax -2. **CSRF protection:** For mutation endpoints (tag/manifest delete) -3. **Rate limiting:** On API endpoints -4. **Input validation:** Sanitize user input for search/filters -5. **Authorization:** Verify authenticated user owns resources before mutation -6. **SQL injection:** Use parameterized queries - -## Performance Considerations - -1. **Database indexes:** On DID, repository, created_at, digest -2. **Pagination:** Limit query results to avoid large payloads -3. **Caching:** Cache profile data, hold list, manifest details -4. **Firehose buffering:** Batch database inserts -5. **Connection pooling:** For SQLite and HTTP clients - -## Testing Strategy - -1. **Unit tests:** Database layer, API handlers -2. **Integration tests:** Firehose worker with mock events -3. **E2E tests:** Full login → browse → manage flow -4. **Load testing:** Firehose worker with high event volume -5. **Manual testing:** Real PDS, real images, real firehose diff --git a/docs/BLUESKY_MANIFEST_POSTS.md b/docs/BLUESKY_MANIFEST_POSTS.md deleted file mode 100644 index 20a9a29..0000000 --- a/docs/BLUESKY_MANIFEST_POSTS.md +++ /dev/null @@ -1,996 +0,0 @@ -# Bluesky Manifest Posts - -## Overview - -This document describes the feature for posting to Bluesky when OCI manifests are uploaded to ATCR holds. When a user pushes an image to the registry, the hold's embedded PDS will: - -1. Create `io.atcr.hold.layer` records for structured metadata tracking -2. Post to Bluesky announcing the push (similar to the "what's new" feed on the AppView web UI) - -## Architecture - -### High-Level Flow - -``` -User pushes image - ↓ -AppView receives manifest PUT request - ↓ -AppView stores manifest in user's PDS - ↓ -AppView notifies hold via XRPC - ↓ -Hold creates layer records in embedded PDS - ↓ -Hold creates Bluesky post - ↓ -Post appears in Bluesky feed -``` - -### Component Interactions - -**AppView** (`pkg/appview/storage/manifest_store.go`): -- After successfully uploading manifest to user's PDS -- Extracts manifest metadata (repository, tag, user info, layers) -- Calls hold's `io.atcr.hold.notifyManifest` XRPC endpoint -- Uses service token from user's PDS for authentication -- Gracefully handles notification failures (doesn't fail manifest upload) - -**Hold** (`pkg/hold/oci/xrpc.go`): -- Receives manifest notification via new XRPC endpoint -- Validates service token and extracts user DID -- Creates layer records for each blob reference in manifest -- Creates Bluesky post announcing the push -- Returns success/failure status - -**Hold's Embedded PDS** (`pkg/hold/pds/`): -- Stores layer records in `io.atcr.hold.layer` collection -- Stores Bluesky posts in `app.bsky.feed.post` collection -- Both are ATProto records with auto-generated TID rkeys -- Queryable via standard ATProto sync endpoints - -## Implementation Details - -### 1. Layer Record Schema - -**File**: `pkg/atproto/lexicon.go` - -**Collection**: `io.atcr.hold.layer` - -**Purpose**: Structured metadata about container layers stored in the hold - -**Schema**: -```go -type LayerRecord struct { - // Type identifier (always "io.atcr.hold.layer") - Type string `json:"$type" cborgen:"$type"` - - // Digest of the layer (e.g., "sha256:abc123...") - Digest string `json:"digest" cborgen:"digest"` - - // Size in bytes - Size int64 `json:"size" cborgen:"size"` - - // MediaType of the layer - MediaType string `json:"mediaType" cborgen:"mediaType"` - - // Repository this layer belongs to (e.g., "alice/myapp") - Repository string `json:"repository" cborgen:"repository"` - - // User DID who uploaded this layer - UserDID string `json:"userDid" cborgen:"userDid"` - - // User handle (for display purposes) - UserHandle string `json:"userHandle,omitempty" cborgen:"userHandle,omitempty"` - - // Timestamp - CreatedAt time.Time `json:"createdAt" cborgen:"createdAt"` -} -``` - -**Constructor**: -```go -func NewLayerRecord(digest string, size int64, mediaType, repository, userDID, userHandle string) *LayerRecord { - return &LayerRecord{ - Type: LayerCollection, - Digest: digest, - Size: size, - MediaType: mediaType, - Repository: repository, - UserDID: userDID, - UserHandle: userHandle, - CreatedAt: time.Now(), - } -} -``` - -**Why CBOR tags**: The hold's embedded PDS uses CBOR encoding for efficient storage in the SQLite-backed carstore. All records stored in the hold must have `cborgen:` tags. - -### 2. XRPC Manifest Notification Endpoint - -**File**: `pkg/hold/oci/xrpc.go` - -**Endpoint**: `POST /xrpc/io.atcr.hold.notifyManifest` - -**Authentication**: Service token from user's PDS (same pattern as blob upload endpoints) - -**Request Schema**: -```go -type NotifyManifestRequest struct { - // Repository name (e.g., "alice/myapp") - Repository string `json:"repository"` - - // Tag (e.g., "latest", "v1.0.0") - Tag string `json:"tag"` - - // User DID (e.g., "did:plc:abc123") - UserDID string `json:"userDid"` - - // User handle (e.g., "alice.bsky.social") - UserHandle string `json:"userHandle"` - - // Manifest content (parsed from uploaded manifest) - Manifest struct { - MediaType string `json:"mediaType"` - Config struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - } `json:"config"` - Layers []struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - MediaType string `json:"mediaType"` - } `json:"layers"` - } `json:"manifest"` -} -``` - -**Response Schema**: -```go -type NotifyManifestResponse struct { - Success bool `json:"success"` - LayersCreated int `json:"layersCreated"` - PostCreated bool `json:"postCreated"` - PostURI string `json:"postUri,omitempty"` // ATProto URI if post created - Error string `json:"error,omitempty"` -} -``` - -**Handler Implementation**: -```go -func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - // 1. Validate service token (reuse existing auth middleware pattern) - userDID, err := h.validateServiceToken(ctx, r) - if err != nil { - writeXRPCError(w, "InvalidToken", err.Error()) - return - } - - // 2. Parse request - var req NotifyManifestRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeXRPCError(w, "InvalidRequest", err.Error()) - return - } - - // 3. Verify user DID matches token - if req.UserDID != userDID { - writeXRPCError(w, "Unauthorized", "user DID mismatch") - return - } - - // 4. Create layer records for each blob - layersCreated := 0 - for _, layer := range req.Manifest.Layers { - record := atproto.NewLayerRecord( - layer.Digest, - layer.Size, - layer.MediaType, - req.Repository, - req.UserDID, - req.UserHandle, - ) - - _, _, err := h.pds.CreateLayerRecord(ctx, record) - if err != nil { - log.Printf("Failed to create layer record: %v", err) - // Continue creating other records - } else { - layersCreated++ - } - } - - // 5. Create Bluesky post - postURI, err := h.pds.CreateManifestPost(ctx, req.Repository, req.Tag, req.UserHandle) - - // 6. Return response - resp := NotifyManifestResponse{ - Success: layersCreated > 0 || err == nil, - LayersCreated: layersCreated, - PostCreated: err == nil, - PostURI: postURI, - } - - if err != nil && layersCreated == 0 { - resp.Error = err.Error() - } - - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(resp) -} -``` - -### 3. Hold PDS Layer Record Methods - -**File**: `pkg/hold/pds/layer.go` (new file) - -**Methods**: - -```go -// CreateLayerRecord creates a new layer record in the hold's PDS -func (p *HoldPDS) CreateLayerRecord(ctx context.Context, record *atproto.LayerRecord) (string, string, error) { - // Validate record - if record.Type != atproto.LayerCollection { - return "", "", fmt.Errorf("invalid record type: %s", record.Type) - } - - if record.Digest == "" { - return "", "", fmt.Errorf("digest is required") - } - - // Create record with auto-generated TID rkey - rkey, recordCID, err := p.repomgr.CreateRecord( - ctx, - p.uid, - atproto.LayerCollection, - record, - ) - - if err != nil { - return "", "", fmt.Errorf("failed to create layer record: %w", err) - } - - log.Printf("Created layer record at %s/%s (digest: %s, size: %d)", - atproto.LayerCollection, rkey, record.Digest, record.Size) - - return rkey, recordCID.String(), nil -} - -// ListLayerRecords lists layer records with optional filtering -func (p *HoldPDS) ListLayerRecords(ctx context.Context, limit int, cursor string) ([]*atproto.LayerRecord, string, error) { - // Implementation using repomgr.GetRecord for pagination - // This would query the carstore and unmarshal layer records - // Return records + next cursor for pagination -} - -// GetLayerRecord retrieves a specific layer record by rkey -func (p *HoldPDS) GetLayerRecord(ctx context.Context, rkey string) (*atproto.LayerRecord, error) { - // Implementation using repomgr.GetRecord -} -``` - -### 4. Bluesky Post Creation with Facets - -**File**: `pkg/hold/pds/manifest_post.go` (new file) - -**Pattern**: Extends `status.go` pattern with rich text facets - -```go -// CreateManifestPost creates a Bluesky post announcing a manifest upload -// Includes facets for clickable mentions and links -func (p *HoldPDS) CreateManifestPost( - ctx context.Context, - repository, tag, userHandle, digest string, - totalSize int64, -) (string, error) { - now := time.Now() - - // Build AppView repository URL - appViewURL := fmt.Sprintf("https://atcr.io/r/%s/%s", userHandle, repository) - - // Format post text components - digestShort := formatDigest(digest) - sizeStr := formatSize(totalSize) - repoWithTag := fmt.Sprintf("%s:%s", repository, tag) - - // Build text: "@alice.bsky.social just pushed hsm-secrets-operator:latest\nDigest: sha256:abc...def Size: 12.2 MB" - text := fmt.Sprintf("@%s just pushed %s\nDigest: %s Size: %s", userHandle, repoWithTag, digestShort, sizeStr) - - // Create facets for mentions and links - facets := buildFacets(text, userHandle, repoWithTag, appViewURL) - - // Create post struct with facets - post := &bsky.FeedPost{ - LexiconTypeID: "app.bsky.feed.post", - Text: text, - Facets: facets, - CreatedAt: now.Format(time.RFC3339), - } - - // Create record with auto-generated TID - rkey, recordCID, err := p.repomgr.CreateRecord( - ctx, - p.uid, - "app.bsky.feed.post", - post, - ) - - if err != nil { - return "", fmt.Errorf("failed to create manifest post: %w", err) - } - - // Build ATProto URI for the post - postURI := fmt.Sprintf("at://%s/app.bsky.feed.post/%s", p.did, rkey) - - log.Printf("Created manifest post: %s (cid: %s)", postURI, recordCID) - - return postURI, nil -} - -// formatDigest truncates digest to first 7 and last 7 chars -// Example: sha256:abc1234567890...fedcba9876543210 -> sha256:abc1234...9876543 -func formatDigest(digest string) string { - if !strings.HasPrefix(digest, "sha256:") { - return digest // Return as-is if not sha256 - } - - hash := strings.TrimPrefix(digest, "sha256:") - if len(hash) <= 14 { - return digest // Too short to truncate - } - - return fmt.Sprintf("sha256:%s...%s", hash[:7], hash[len(hash)-7:]) -} - -// formatSize converts bytes to human-readable format -// Examples: 1024 -> "1.0 KB", 1048576 -> "1.0 MB", 1073741824 -> "1.0 GB" -func formatSize(bytes int64) string { - const ( - KB = 1024 - MB = 1024 * KB - GB = 1024 * MB - ) - - switch { - case bytes >= GB: - return fmt.Sprintf("%.1f GB", float64(bytes)/float64(GB)) - case bytes >= MB: - return fmt.Sprintf("%.1f MB", float64(bytes)/float64(MB)) - case bytes >= KB: - return fmt.Sprintf("%.1f KB", float64(bytes)/float64(KB)) - default: - return fmt.Sprintf("%d B", bytes) - } -} - -// buildFacets creates mention and link facets for rich text -// IMPORTANT: Byte offsets must be calculated for UTF-8 encoded text -func buildFacets(text, userHandle, repoWithTag, appViewURL string) []*bsky.RichtextFacet { - facets := []*bsky.RichtextFacet{} - - // Find mention: "@alice.bsky.social" - mentionText := "@" + userHandle - mentionStart := strings.Index(text, mentionText) - if mentionStart >= 0 { - // Calculate byte offsets (not character offsets!) - byteStart := int64(len(text[:mentionStart])) - byteEnd := int64(len(text[:mentionStart+len(mentionText)])) - - facets = append(facets, &bsky.RichtextFacet{ - Index: &bsky.RichtextFacet_ByteSlice{ - ByteStart: byteStart, - ByteEnd: byteEnd, - }, - Features: []*bsky.RichtextFacet_Features_Elem{ - { - RichtextFacet_Mention: &bsky.RichtextFacet_Mention{ - Did: "", // Will be resolved by Bluesky from handle - }, - }, - }, - }) - } - - // Find repository link: "hsm-secrets-operator:latest" - linkStart := strings.Index(text, repoWithTag) - if linkStart >= 0 { - // Calculate byte offsets - byteStart := int64(len(text[:linkStart])) - byteEnd := int64(len(text[:linkStart+len(repoWithTag)])) - - facets = append(facets, &bsky.RichtextFacet{ - Index: &bsky.RichtextFacet_ByteSlice{ - ByteStart: byteStart, - ByteEnd: byteEnd, - }, - Features: []*bsky.RichtextFacet_Features_Elem{ - { - RichtextFacet_Link: &bsky.RichtextFacet_Link{ - Uri: appViewURL, - }, - }, - }, - }) - } - - return facets -} -``` - -**Facet Implementation Notes:** - -1. **Byte Offsets**: ATProto uses byte offsets (UTF-8 encoded), not character offsets - - For ASCII text: `len(text[:index])` gives correct byte offset - - For Unicode: Must use `len()` on substring to get byte count - - Never use `rune` indexes directly - -2. **Mention Facets**: - - Include `@` symbol in the facet range - - DID field can be empty; Bluesky resolves from handle - - Type: `app.bsky.richtext.facet#mention` - -3. **Link Facets**: - - Text can be anything (doesn't have to be URL) - - URI field contains actual target URL - - Type: `app.bsky.richtext.facet#link` - -4. **Ordering**: Facets should not overlap; order doesn't matter - -### 5. AppView Integration - -**File**: `pkg/appview/storage/manifest_store.go` - -**Integration Point**: After `client.PutRecord()` succeeds (around line 130-140) - -```go -// Existing code: -recordURI, recordCID, err := ms.client.PutRecord(ctx, atproto.ManifestCollection, rkey, manifestRecord) -if err != nil { - return "", fmt.Errorf("failed to store manifest in PDS: %w", err) -} - -// NEW: Notify hold about manifest upload -if err := ms.notifyHoldAboutManifest(ctx, desc, manifestRecord, tag); err != nil { - // Log error but don't fail the manifest upload - log.Printf("Failed to notify hold about manifest: %v", err) -} - -return desc.Digest.String(), nil -``` - -**Implementation**: - -```go -// notifyHoldAboutManifest sends manifest metadata to the hold -func (ms *ManifestStore) notifyHoldAboutManifest( - ctx context.Context, - desc distribution.Descriptor, - manifestRecord *atproto.ManifestRecord, - tag string, -) error { - // 1. Get registry context - regCtx, err := storage.GetRegistryContext(ctx) - if err != nil { - return fmt.Errorf("failed to get registry context: %w", err) - } - - // 2. Resolve hold DID to endpoint - holdEndpoint, err := ms.resolver.ResolveDIDToHTTPEndpoint(ctx, manifestRecord.HoldDID) - if err != nil { - return fmt.Errorf("failed to resolve hold DID: %w", err) - } - - // 3. Get service token from user's PDS - serviceToken, err := regCtx.Refresher.GetServiceToken(ctx, regCtx.DID, manifestRecord.HoldDID) - if err != nil { - return fmt.Errorf("failed to get service token: %w", err) - } - - // 4. Parse manifest to extract layer info - var parsedManifest struct { - MediaType string `json:"mediaType"` - Config distribution.Descriptor `json:"config"` - Layers []distribution.Descriptor `json:"layers"` - } - - if err := json.Unmarshal(manifestRecord.ManifestBlob.Data, &parsedManifest); err != nil { - return fmt.Errorf("failed to parse manifest: %w", err) - } - - // 5. Build notification request - notifyReq := map[string]any{ - "repository": ms.repository, - "tag": tag, - "userDid": regCtx.DID, - "userHandle": regCtx.Handle, // Need to add this to RegistryContext - "manifest": map[string]any{ - "mediaType": parsedManifest.MediaType, - "config": map[string]any{ - "digest": parsedManifest.Config.Digest.String(), - "size": parsedManifest.Config.Size, - }, - "layers": func() []map[string]any { - layers := make([]map[string]any, len(parsedManifest.Layers)) - for i, layer := range parsedManifest.Layers { - layers[i] = map[string]any{ - "digest": layer.Digest.String(), - "size": layer.Size, - "mediaType": layer.MediaType, - } - } - return layers - }(), - }, - } - - // 6. Call hold's XRPC endpoint - reqBody, _ := json.Marshal(notifyReq) - req, err := http.NewRequestWithContext( - ctx, - "POST", - holdEndpoint+"/xrpc/io.atcr.hold.notifyManifest", - bytes.NewReader(reqBody), - ) - if err != nil { - return err - } - - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+serviceToken) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("hold notification failed: %s (status: %d)", body, resp.StatusCode) - } - - // 7. Parse response (optional logging) - var notifyResp map[string]any - if err := json.NewDecoder(resp.Body).Decode(¬ifyResp); err == nil { - log.Printf("Hold notification successful: %+v", notifyResp) - } - - return nil -} -``` - -### 6. Record Type Registration - -**File**: `pkg/hold/pds/server.go` - -**In `init()` function** (around line 30): - -```go -func init() { - // Existing registrations - lexutil.RegisterType(atproto.CaptainCollection, &atproto.CaptainRecord{}) - lexutil.RegisterType(atproto.CrewCollection, &atproto.CrewRecord{}) - lexutil.RegisterType(atproto.TangledProfileCollection, &atproto.TangledProfileRecord{}) - - // NEW: Register layer record type - lexutil.RegisterType(atproto.LayerCollection, &atproto.LayerRecord{}) -} -``` - -**Why needed**: ATProto's CBOR unmarshaling requires type registration to automatically deserialize records when reading from the carstore. - -## Testing Strategy - -### Unit Tests - -**Test Layer Record Creation** (`pkg/hold/pds/layer_test.go`): -```go -func TestCreateLayerRecord(t *testing.T) { - pds := setupTestPDS(t) - ctx := context.Background() - - record := atproto.NewLayerRecord( - "sha256:abc123", - 1024, - "application/vnd.docker.image.rootfs.diff.tar.gzip", - "alice/myapp", - "did:plc:alice123", - "alice.bsky.social", - ) - - rkey, cid, err := pds.CreateLayerRecord(ctx, record) - assert.NoError(t, err) - assert.NotEmpty(t, rkey) - assert.NotEmpty(t, cid) - - // Verify record was stored - retrieved, err := pds.GetLayerRecord(ctx, rkey) - assert.NoError(t, err) - assert.Equal(t, record.Digest, retrieved.Digest) -} -``` - -**Test Manifest Post Creation** (`pkg/hold/pds/manifest_post_test.go`): -```go -func TestCreateManifestPost(t *testing.T) { - pds := setupTestPDS(t) - ctx := context.Background() - - postURI, err := pds.CreateManifestPost(ctx, "alice/myapp", "latest", "alice.bsky.social") - assert.NoError(t, err) - assert.Contains(t, postURI, "app.bsky.feed.post") - - // Parse URI and verify post exists - // at://did:web:hold01.atcr.io/app.bsky.feed.post/{rkey} -} -``` - -**Test XRPC Endpoint** (`pkg/hold/oci/xrpc_test.go`): -```go -func TestHandleNotifyManifest(t *testing.T) { - handler := setupTestHandler(t) - - req := NotifyManifestRequest{ - Repository: "alice/myapp", - Tag: "latest", - UserDID: "did:plc:alice123", - UserHandle: "alice.bsky.social", - Manifest: /* ... */, - } - - // Make HTTP request with service token - resp := makeRequest(t, handler, req, validServiceToken) - - assert.Equal(t, http.StatusOK, resp.StatusCode) - - var result NotifyManifestResponse - json.NewDecoder(resp.Body).Decode(&result) - - assert.True(t, result.Success) - assert.Equal(t, 3, result.LayersCreated) // if manifest has 3 layers - assert.True(t, result.PostCreated) -} -``` - -### Integration Tests - -**End-to-End Test**: -1. Push a test image to ATCR -2. Verify manifest is stored in user's PDS -3. Verify layer records are created in hold's PDS -4. Verify Bluesky post is created in hold's PDS -5. Query ATProto endpoints to retrieve records - -## Error Handling - -### AppView Side - -**Notification failures should NOT break manifest uploads**: -- If hold is unreachable: Log error, continue -- If service token fails: Log error, continue -- If hold returns error: Log error, continue - -**Rationale**: Bluesky posts are a "nice to have" feature, not critical infrastructure. Image pushes must succeed even if social features fail. - -### Hold Side - -**Partial failures are acceptable**: -- If some layer records fail: Create what we can, return partial success -- If Bluesky post fails but layers succeed: Return success with `postCreated: false` -- If all operations fail: Return error response - -**Logging**: -- Log all errors for debugging -- Include user DID, repository, and error details -- Use structured logging for easy querying - -## Configuration - -### Environment Variables - -**Hold Service** (`.env.hold.example`): -```bash -# Enable/disable Bluesky manifest posting (default: false) -# When enabled, hold will create Bluesky posts when users push images -# Synced to captain record's enableBlueskyPosts field on startup -HOLD_BLUESKY_POSTS_ENABLED=false -``` - -**AppView** - No configuration needed. AppView always attempts to notify holds after manifest uploads, but handles failures gracefully. - -### Feature Flags - -**Captain Record Sync:** -The hold's captain record includes an `enableBlueskyPosts` field that is synchronized with the environment variable on startup: - -```go -type CaptainRecord struct { - // ... other fields ... - EnableBlueskyPosts bool `json:"enableBlueskyPosts" cborgen:"enableBlueskyPosts"` -} -``` - -**How it works:** -1. On startup, Bootstrap reads `HOLD_BLUESKY_POSTS_ENABLED` environment variable -2. Creates or updates the captain record to match the env var setting -3. At runtime, the code reads from the captain record (which reflects the env var) -4. To change the setting, update the env var and restart the hold - -**Rationale:** -- Default off for backward compatibility and privacy -- Hold owners can enable via env var at deployment -- Per-hold override via captain record for multi-tenant scenarios -- Follows same pattern as existing status post feature - -## Performance Considerations - -### Database Impact - -**Layer records**: Each manifest upload creates N records (where N = number of layers) -- Typical image: 5-10 layers -- Large image: 50+ layers -- Storage: ~500 bytes per record (CBOR compressed) - -**Bluesky posts**: One post per manifest -- Storage: ~200 bytes per post -- Indexed by creation time for feed queries - -**Carstore growth**: Estimate ~5KB per manifest upload (records + post) - -### Network Impact - -**AppView → Hold notification**: -- One HTTP POST per manifest upload -- Payload size: ~2-10KB (depends on layer count) -- Should complete in <100ms on local network - -**Service token requests**: -- Tokens cached for 50 seconds -- Minimal overhead if pushing multiple manifests quickly - -### Optimization Opportunities - -1. **Batch layer record creation**: Use `BatchWrite` for multiple records -2. **Async processing**: Queue notifications and process in background -3. **Rate limiting**: Limit posts per user/hold to prevent spam -4. **Deduplication**: Skip layer records for already-seen digests - -## Future Enhancements - -### Phase 2: Enhanced Posts - -**Rich embeds**: -- Link preview to AppView repository page -- Thumbnail image from first layer -- Metadata badges (image size, layer count, tags) - -**Mentions**: -- Parse user handle and create Bluesky facets for @mentions -- Enable clickable mentions in posts - -**Tags/hashtags**: -- Add `#container`, `#docker`, repository tags -- Improve discoverability in Bluesky - -### Phase 3: Feed Customization - -**Hold-specific feeds**: -- Query layer records by repository -- Filter by user DID -- Time-based queries - -**ATProto feed generator**: -- Implement `app.bsky.feed.getFeedSkeleton` XRPC endpoint -- Publish hold's feed to Bluesky -- Users can subscribe to hold activity feeds - -### Phase 4: Analytics - -**Track metrics**: -- Posts per day/week/month -- Most active users -- Most popular repositories -- Storage growth over time - -**Dashboards**: -- Visualize activity on AppView UI -- Show trending images -- Leaderboards for most pushed repositories - -## Security Considerations - -### Authentication - -**Service tokens**: -- Validate tokens against user's PDS -- Verify DID matches in token claims -- Check token expiration (60s from PDS) - -**Authorization**: -- Only authenticated users can trigger posts -- Posts created under hold's DID (not user's DID) -- User information is metadata in post text - -### Privacy - -**User handles**: -- Posts include user handle (`@alice.bsky.social`) -- Consider opt-out mechanism for privacy-conscious users - -**Repository names**: -- Public information (already visible in AppView) -- Consider private repository flags in future - -### Rate Limiting - -**Prevent spam**: -- Limit posts per user per hour -- Detect rapid-fire pushes (CI/CD) -- Consider aggregating multiple pushes into single post - -**Resource protection**: -- Limit layer record creation to prevent storage exhaustion -- Cap manifest notification payload size -- Timeout long-running operations - -## Monitoring and Observability - -### Metrics to Track - -**AppView**: -- `atcr_hold_notifications_total` - Counter of notifications sent -- `atcr_hold_notifications_errors` - Counter of failures -- `atcr_hold_notification_duration_ms` - Histogram of latency - -**Hold**: -- `hold_layer_records_created_total` - Counter of layer records -- `hold_bluesky_posts_created_total` - Counter of posts -- `hold_manifest_notifications_received_total` - Counter of incoming notifications -- `hold_notification_errors_total` - Counter of errors by type - -### Logging - -**Structured logs**: -```json -{ - "level": "info", - "msg": "manifest notification received", - "repository": "alice/myapp", - "tag": "latest", - "userDid": "did:plc:alice123", - "layerCount": 5, - "layersCreated": 5, - "postCreated": true, - "duration_ms": 45 -} -``` - -### Alerts - -**Critical issues**: -- High error rate (>10% failures) -- Service token failures (auth issues) -- PDS carstore errors (database problems) - -**Warning issues**: -- Slow notifications (>1s latency) -- Partial failures (some layers not created) -- Missing user handle in context - -## Migration Strategy - -### Rollout Plan - -**Phase 1: Development** -- Implement core functionality -- Add comprehensive tests -- Deploy to staging environment - -**Phase 2: Beta** -- Enable for test holds only -- Gather feedback from early users -- Monitor performance and errors - -**Phase 3: Opt-in** -- Add configuration flags -- Allow hold owners to enable feature -- Document setup process - -**Phase 4: Default On** -- Enable by default for new holds -- Migrate existing holds (opt-out available) -- Announce feature publicly - -### Backward Compatibility - -**No breaking changes**: -- New XRPC endpoint (doesn't affect existing endpoints) -- New record types (isolated collections) -- Optional feature (can be disabled) - -**Existing holds**: -- Work without changes -- Can opt-in by updating hold service -- No data migration required - -## Example Post Formats - -### Preferred Format (Facet-Based) - -**Text representation:** -``` -@alice.bsky.social just pushed hsm-secrets-operator:latest -Digest: sha256:abc1234...def5678 Size: 12.2 MB -``` - -**Actual implementation:** -- `@alice.bsky.social` - Clickable mention (facet type: `app.bsky.richtext.facet#mention`) -- `hsm-secrets-operator:latest` - Clickable link to `https://atcr.io/r/alice.bsky.social/hsm-secrets-operator` (facet type: `app.bsky.richtext.facet#link`) -- `sha256:abc1234...def5678` - Truncated digest (first 7 + last 7 chars) -- `12.2 MB` - Human-readable size (auto-formatted from bytes) - -**Why facets?** -- Mentions are clickable and link to user profiles in Bluesky -- Repository names link directly to AppView repository pages -- Better user experience than plain text URLs -- Standard ATProto rich text format - -### Alternative Formats - -#### Simple Format -``` -📦 alice/myapp:latest pushed by @alice.bsky.social -``` - -#### Detailed Format -``` -📦 New container image pushed! - -alice/myapp:v1.2.3 -Pushed by @alice.bsky.social -5 layers, 125 MB total - -View: https://atcr.io/alice/myapp -``` - -#### With Emoji/Styling -``` -🚀 alice/myapp:latest - -✅ 5 layers -📦 125.4 MB -👤 @alice.bsky.social -🔗 atcr.io/alice/myapp -``` - -#### With Tags -``` -📦 alice/myapp:latest pushed by @alice.bsky.social - -#container #docker #atcr -``` - -## References - -### Related Code - -- Existing Bluesky post implementation: `pkg/hold/pds/status.go` -- XRPC endpoint pattern: `pkg/hold/oci/xrpc.go` -- Record type definitions: `pkg/atproto/lexicon.go` -- Manifest storage: `pkg/appview/storage/manifest_store.go` -- Service token handling: `pkg/auth/oauth/refresher.go` - -### External Documentation - -- ATProto Record Schema: https://atproto.com/specs/record-key -- Bluesky Post Lexicon: https://atproto.com/lexicons/app-bsky-feed#appbskyfeedpost -- CBOR Encoding: https://cbor.io/ -- Bluesky Facets (mentions/links): https://atproto.com/specs/richtext - -### Tools - -- CBOR code generation: `github.com/whyrusleeping/cbor-gen` -- ATProto libraries: `github.com/bluesky-social/indigo` -- Testing: Standard Go testing + `testify/assert` diff --git a/docs/CREW_ACCESS_CONTROL.md b/docs/CREW_ACCESS_CONTROL.md deleted file mode 100644 index b541852..0000000 --- a/docs/CREW_ACCESS_CONTROL.md +++ /dev/null @@ -1,250 +0,0 @@ -# Hold Crew Access Control - -## Overview - -ATCR uses crew-based access control for hold (storage) services. Crew records are stored in the **hold's embedded PDS** (not the owner's or user's PDS), making the hold a self-contained ATProto actor with its own access control. - -## Current Implementation - -### Records in Hold's PDS - -**Captain record** - Hold ownership (single record at `io.atcr.hold.captain/self`): -```json -{ - "$type": "io.atcr.hold.captain", - "owner": "did:plc:alice123", - "public": false, - "deployedAt": "2025-10-14T...", - "region": "iad", - "provider": "fly.io" -} -``` - -**Crew records** - Access control (one per member at `io.atcr.hold.crew/{rkey}`): -```json -{ - "$type": "io.atcr.hold.crew", - "member": "did:plc:bob456", - "role": "admin", - "permissions": ["blob:read", "blob:write"], - "addedAt": "2025-10-14T..." -} -``` - -### Authorization Logic - -Write authorization follows this priority: - -``` -isAuthorizedWrite(userDID): - 1. If userDID == captain.owner → ALLOW - 2. If crew record exists for userDID → ALLOW - 3. Default → DENY -``` - -Read authorization depends on `HOLD_PUBLIC` setting: -- **Public hold** (`HOLD_PUBLIC=true`): Anonymous + all authenticated users can read -- **Private hold** (`HOLD_PUBLIC=false`): Requires crew membership for reads - -### Configuration - -```bash -# Access control environment variables -HOLD_PUBLIC=false # Require authentication for reads -HOLD_ALLOW_ALL_CREW=false # Only explicit crew members can write -``` - -### Crew Management - -Crew records are managed by the hold captain (owner) using standard ATProto operations on the hold's embedded PDS: - -**Add crew member:** -```bash -# Via hold's PDS (requires captain's OAuth) -atproto put-record \ - --pds https://hold.example.com \ - --collection io.atcr.hold.crew \ - --rkey "{memberDID}" \ - --value '{ - "$type": "io.atcr.hold.crew", - "member": "did:plc:bob456", - "role": "admin", - "permissions": ["blob:read", "blob:write"], - "addedAt": "2025-10-14T12:00:00Z" - }' -``` - -**Remove crew member:** -```bash -atproto delete-record \ - --pds https://hold.example.com \ - --collection io.atcr.hold.crew \ - --rkey "{memberDID}" -``` - -**List crew members:** -```bash -# Via XRPC -GET https://hold.example.com/xrpc/com.atproto.repo.listRecords?repo={holdDID}&collection=io.atcr.hold.crew -``` - -## Authentication Flow - -``` -1. User pushes image to atcr.io/alice/myapp - -2. AppView gets service token from alice's PDS: - GET /xrpc/com.atproto.server.getServiceAuth?aud={holdDID} - Response: { "token": "..." } - -3. AppView calls hold with service token: - POST /xrpc/io.atcr.hold.initiateUpload - Authorization: Bearer {serviceToken} - -4. Hold validates service token: - - Checks token is from alice's PDS - - Extracts alice's DID from token - -5. Hold checks crew membership: - - Queries its own PDS: com.atproto.repo.getRecord - - Collection: io.atcr.hold.crew - - Record key: alice's DID - -6. If crew record found → allow upload - Else → deny with 403 Forbidden -``` - -**Trust model:** "Trust but verify" -- User OAuth'd to AppView (proves identity) -- Service token from user's PDS (proves AppView is acting on behalf of user) -- Crew record in hold's PDS (proves user has access to this hold) - -## Use Cases - -### 1. Personal Hold (Private) - -```bash -# Owner only -HOLD_PUBLIC=false -HOLD_ALLOW_ALL_CREW=false -# No additional crew records needed - captain has implicit access -``` - -### 2. Team Hold (Shared) - -```bash -# Multiple team members -HOLD_PUBLIC=false -HOLD_ALLOW_ALL_CREW=false - -# Captain adds crew members: -# - did:plc:alice (admin) -# - did:plc:bob (member) -# - did:plc:charlie (member) -``` - -### 3. Public Hold (Community) - -```bash -# Allow any authenticated user (TODO: Implement HOLD_ALLOW_ALL_CREW) -HOLD_PUBLIC=true -HOLD_ALLOW_ALL_CREW=true -``` - -## Planned Features - -### Pattern-Based Access Control - -**Status:** Planned but not yet implemented. - -**Concept:** Allow crew records with pattern matching instead of explicit DIDs: - -```json -{ - "$type": "io.atcr.hold.crew", - "memberPattern": "*.example.com", - "role": "write" -} -``` - -**Use cases:** -- `"*"` - Allow all authenticated users -- `"*.company.com"` - Allow all users from company domain -- `"*.community.social"` - Allow all community members - -**Implementation needed:** -- Add `memberPattern` field to crew record schema (make `member` optional) -- Add handle resolution (DID → handle lookup) -- Add pattern matching logic -- Update authorization to check patterns - -### Barred List (Access Revocation) - -**Status:** Planned but not yet implemented. - -**Concept:** Explicit deny list that overrides crew membership: - -```json -{ - "$type": "io.atcr.hold.crew.barred", - "member": "did:plc:former-employee", - "reason": "No longer with company", - "barredAt": "2025-10-13T12:00:00Z" -} -``` - -**Priority:** Barred list checked before crew list. - -### HOLD_ALLOW_ALL_CREW - -**Status:** Environment variable exists but full implementation pending. - -**Concept:** Automatically create/manage wildcard crew record via env var: - -```bash -HOLD_ALLOW_ALL_CREW=true # Creates crew record with memberPattern: "*" -``` - -**Implementation needed:** -- Auto-create wildcard crew record on startup if env=true -- Auto-delete wildcard crew record if env changes to false -- Use well-known rkey "allow-all" for managed record - -## Architecture Notes - -### Why Hold's Embedded PDS? - -**Key insight:** Crew records are **shared data** about the hold, not user-specific data. - -**Benefits:** -- **Self-contained**: Hold is independent ATProto actor -- **Portable**: Hold can move without coordinating with user PDSs -- **Discoverable**: Query hold's PDS to see who has access -- **Standard**: Uses normal ATProto sync endpoints (subscribeRepos, getRecord, listRecords) - -**Comparison:** -- **User's PDS**: Stores user-specific data (manifests, sailor profile) -- **Hold's PDS**: Stores hold-specific data (captain, crew, configuration) -- Clear separation of concerns - -### Security Considerations - -1. **Public Records**: Crew records are public (anyone can see who has access to a hold) -2. **Service Tokens**: Hold trusts user's PDS to issue valid service tokens -3. **DID-Based**: Crew membership is DID-based (permanent), not handle-based -4. **Captain Control**: Only captain can modify crew records (via OAuth to hold's PDS) - -## Future Improvements - -1. **Crew management UI** - Web interface for adding/removing crew members -2. **Pattern-based matching** - Implement `memberPattern` field -3. **Barred list** - Implement access revocation -4. **Role-based permissions** - Fine-grained permissions beyond read/write -5. **Temporary access** - Time-limited crew membership (`expiresAt` field) -6. **Audit logging** - Track access grants/denials - -## References - -- [EMBEDDED_PDS.md](./EMBEDDED_PDS.md) - Embedded PDS architecture details -- [BYOS.md](./BYOS.md) - BYOS deployment and usage -- [ATProto Lexicon Spec](https://atproto.com/specs/lexicon) diff --git a/docs/EMBEDDED_PDS.md b/docs/EMBEDDED_PDS.md deleted file mode 100644 index 4d9c6eb..0000000 --- a/docs/EMBEDDED_PDS.md +++ /dev/null @@ -1,355 +0,0 @@ -# Embedded PDS Architecture for Hold Services - -This document describes ATCR's hold service architecture using embedded ATProto PDS (Personal Data Server) for access control and federation. - -## Motivation - -### The Fragmentation Problem - -Several ATProto projects face similar challenges with large data storage: - -| Project | Large Data | Metadata | Solution | -|---------|-----------|----------|----------| -| **tangled.org** | Git objects | Issues, PRs, comments | External knot storage | -| **stream.place** | Video segments | Stream info, chat | Embedded "static PDS" | -| **ATCR** | Container blobs | Manifests, comments, builds | Embedded PDS in hold service | - -**Common problem:** Large binary data can't realistically live in user PDSs, but application metadata needs a distributed home. - -**ATCR's approach:** Each hold service is a full ATProto actor with its own embedded PDS for **shared data** (captain + crew records, not user-specific data). This PDS stores access control and metadata about the hold itself. - -## Current Architecture - -### Hold Service Components - -``` -Hold Service (did:web:hold01.atcr.io) -├── Embedded PDS (SQLite carstore) - Shared data only -│ ├── Captain record (ownership metadata) -│ ├── Crew records (access control) -│ └── ATProto sync/repo endpoints -├── OCI multipart upload (XRPC) -│ ├── io.atcr.hold.initiateUpload -│ ├── io.atcr.hold.getPartUploadUrl -│ ├── io.atcr.hold.uploadPart -│ ├── io.atcr.hold.completeUpload -│ └── io.atcr.hold.abortUpload -└── Storage driver (S3, filesystem, etc.) -``` - -**Important distinction:** -- **Hold's embedded PDS** = Shared data (crew members, hold configuration) -- **User's PDS** = User-specific data (manifests, sailor profile, personal records) -- Hold's PDS does NOT store user-specific container data (that stays in user's own PDS) - -### Records Structure - -**Captain record** (hold ownership, single record at `io.atcr.hold.captain/self`): -```json -{ - "$type": "io.atcr.hold.captain", - "owner": "did:plc:alice123", - "public": false, - "deployedAt": "2025-10-14T...", - "region": "iad", - "provider": "fly.io" -} -``` - -**Crew records** (access control, one per member at `io.atcr.hold.crew/{rkey}`): -```json -{ - "$type": "io.atcr.hold.crew", - "member": "did:plc:bob456", - "role": "admin", - "permissions": ["blob:read", "blob:write"], - "addedAt": "2025-10-14T..." -} -``` - -### ATProto PDS Endpoints - -Standard ATProto sync endpoints: -- `GET /xrpc/com.atproto.sync.getRepo` - Download repository as CAR file -- `GET /xrpc/com.atproto.sync.getBlob` - Get blob or presigned download URL -- `GET /xrpc/com.atproto.sync.subscribeRepos` - Real-time crew changes -- `GET /xrpc/com.atproto.sync.listRepos` - List repositories - -Repository management: -- `GET /xrpc/com.atproto.repo.describeRepo` - Repository metadata -- `GET /xrpc/com.atproto.repo.getRecord` - Get specific record (captain/crew) -- `GET /xrpc/com.atproto.repo.listRecords` - List crew members -- `POST /xrpc/io.atcr.hold.requestCrew` - Request crew membership - -DID resolution: -- `GET /.well-known/did.json` - DID document (did:web resolution) -- `GET /.well-known/atproto-did` - DID for handle resolution - -### OCI Multipart Upload Flow - -``` -1. AppView gets service token from user's PDS: - GET /xrpc/com.atproto.server.getServiceAuth?aud={holdDID} - Response: { "token": "eyJ..." } - -2. AppView initiates multipart upload: - POST /xrpc/io.atcr.hold.initiateUpload - Authorization: Bearer {serviceToken} - Body: { "digest": "sha256:abc..." } - Response: { "uploadId": "xyz" } - -3. For each part: - POST /xrpc/io.atcr.hold.getPartUploadUrl - Body: { "uploadId": "xyz", "partNumber": 1 } - Response: { "url": "https://s3.../presigned" } - -4. Upload part to S3 presigned URL: - PUT {presignedURL} - Body: [part data] - -5. Complete upload: - POST /xrpc/io.atcr.hold.completeUpload - Body: { "uploadId": "xyz", "digest": "sha256:abc...", "parts": [...] } -``` - -## Implementation Details - -### Storage: Indigo Carstore with SQLite - -```go -type HoldPDS struct { - did string - carstore carstore.CarStore - session *carstore.DeltaSession // Provides blockstore interface - repo *repo.Repo - dbPath string - uid models.Uid // User ID for carstore (fixed: 1) -} -``` - -**Storage location:** Single SQLite file (`/var/lib/atcr-hold/hold.db`) -- Contains MST nodes, records, commits in carstore tables -- Handles compaction/cleanup automatically -- Migration path to Postgres if needed (same carstore API) - -### Key Implementation Lessons - -#### 1. Custom Record Types Need Manual CBOR Decoding - -```go -// ❌ WRONG - Fails with "unrecognized lexicon type" -record, err := repo.GetRecord(ctx, path, &CrewRecord{}) - -// ✅ CORRECT - Manual CBOR decoding -recordCID, recBytes, err := repo.GetRecordBytes(ctx, path) -var crewRecord CrewRecord -err = crewRecord.UnmarshalCBOR(bytes.NewReader(*recBytes)) -``` - -Indigo's lexicon system doesn't know about custom types like `io.atcr.hold.crew`. - -#### 2. JSON and CBOR Struct Tags Must Match - -```go -// ✅ CORRECT - JSON tags match CBOR tags -type CrewRecord struct { - Type string `json:"$type" cborgen:"$type"` - Member string `json:"member" cborgen:"member"` - Role string `json:"role" cborgen:"role"` - Permissions []string `json:"permissions" cborgen:"permissions"` - AddedAt string `json:"addedAt" cborgen:"addedAt"` -} -``` - -CID verification requires identical bytes from JSON and CBOR encodings. - -#### 3. MST ForEach Returns Full Paths - -```go -// ✅ CORRECT - Extract just the rkey -err := repo.ForEach(ctx, "io.atcr.hold.crew", func(k string, v cid.Cid) error { - // k = "io.atcr.hold.crew/3m37dr2ddit22" - parts := strings.Split(k, "/") - rkey := parts[len(parts)-1] // "3m37dr2ddit22" - return nil -}) -``` - -#### 4. CAR Files Must Include Full MST Path - -For `com.atproto.sync.getRecord`, return CAR with: -1. **Commit block** - Repo head with signature -2. **MST tree nodes** - Path from root to record -3. **Record block** - The actual record data - -Use `util.NewLoggingBstore()` to capture all accessed blocks. - -## IAM Challenges - -### Current Implementation: Service Tokens - -AppView uses `com.atproto.server.getServiceAuth` to get tokens for calling holds: - -```go -// AppView requests service token from user's PDS -GET /xrpc/com.atproto.server.getServiceAuth?aud={holdDID}&lxm=com.atproto.repo.getRecord - -// PDS returns short-lived token (60 seconds) -{ "token": "eyJ..." } - -// AppView uses token to authenticate to hold -Authorization: Bearer eyJ... -``` - -### Known Issues - -#### 1. RPC Permission Format with IP Addresses - -**Problem:** Service token RPC permissions don't work with IP addresses in the audience (`aud`) field: - -``` -Error: RPC permission format invalid -Permission: rpc:com.atproto.repo.getRecord?aud=172.28.0.3:8080#atcr_hold -Issue: IP address with port not supported in aud field -``` - -**Impact:** Local development with IP-based hold DIDs (e.g., `did:web:172.28.0.3:8080`) fails. - -**Workaround:** Falls back to unauthenticated requests (works for public holds only) or use hostname-based DIDs. - -#### 2. Dynamic Hold Discovery Limitation - -**Problem:** AppView can only OAuth a user's default hold (configured in AppView), not dynamically discovered holds from sailor profiles. - -**Current limitation:** -- User sets `defaultHold = "did:web:alice-storage.fly.dev"` in sailor profile -- AppView discovers hold DID when user pushes -- AppView tries to get service token for alice's hold from user's PDS -- BUT: User never OAuth'd through alice's hold, only through AppView's default hold -- Result: No service token available, can't authenticate to alice's hold - -**Why this matters:** -- Users can't seamlessly use BYOS (Bring Your Own Storage) -- Hold references in sailor profiles are non-functional -- Limits portability and decentralization goals - -#### 3. Trust Model: "Trust but Verify" - -**Current approach:** -1. User OAuth's to AppView (credential helper flow) -2. Hold has crew member record for user (authorization) -3. AppView requests service token from user's PDS (proof) -4. Hold validates service token from user's PDS (verification) - -**Philosophy:** "Trust but verify" -- IF user OAuth'd to AppView AND hold has crew member record for user → generally trust -- BUT don't want AppView to lie → need proof from user's PDS that it's actually them -- Service tokens provide this proof (user's PDS says "yes, I authorized this") - -**Challenge:** Service tokens work for this model, but scope/permission format issues (see #1, #2) make it fragile in practice. - -### Potential Solutions - -#### Option A: Direct User-to-Hold Authentication (NOT IMPLEMENTED) - -**Note:** This option was considered but NOT implemented. ATCR uses service tokens exclusively for AppView→Hold authentication. - -Users would authenticate directly to holds (bypassing AppView service tokens). - -**Pros:** -- ✅ Clear trust model (user ↔ hold) -- ✅ Works with any hold (BYOS friendly) -- ✅ No OAuth scope issues - -**Cons:** -- ❌ Multiple OAuth flows (user's PDS + each hold) -- ❌ Complex credential management -- ❌ Poor UX (authenticate to each hold separately) - -#### Option B: AppView as OAuth Client - -AppView pre-registers with holds and uses its own credentials (not user's). - -**Pros:** -- ✅ No OAuth scope issues -- ✅ Single OAuth flow for user -- ✅ Simpler credential management - -**Cons:** -- ❌ Holds must trust AppView (centralization) -- ❌ Doesn't work for unknown holds -- ❌ Requires registration process - -#### Option C: Public Hold API - -Simplify by making holds public for reads, auth only for writes. - -**Pros:** -- ✅ No OAuth complexity for reads -- ✅ Works offline (no PDS dependency) - -**Cons:** -- ❌ Private holds still need auth -- ❌ Not standard ATProto pattern - -#### Option D: Hybrid Service Token + API Key - -Use service tokens when available, fall back to API keys for BYOS holds. - -**Pros:** -- ✅ Optimal for default holds -- ✅ BYOS works with API keys -- ✅ Backward compatible - -**Cons:** -- ❌ Two auth mechanisms -- ❌ Not pure ATProto - -### Recommended Approach - -**Short-term (MVP):** -1. Public holds (no auth needed for reads) -2. Default hold with service tokens (AppView-managed) -3. Document BYOS limitation - -**Medium-term:** -1. Hybrid approach (service tokens + API key fallback) -2. Clear security model for hold operators - -**Long-term:** -1. Continue using service tokens (current implementation) -2. Explore optimizations for service token caching -3. Document security model more clearly - -### Understanding getServiceAuth - -**Purpose:** `com.atproto.server.getServiceAuth` gives a JWT to a service with access to specific functions in the user's PDS. It's a **temporary grant to a service outside of what you OAuth'd to**. - -**How ATCR uses it:** -- User OAuth's to AppView (gets broad access to their account) -- AppView needs to prove to hold that user authorized it -- AppView calls user's PDS: "give me a token scoped for this hold" -- User's PDS issues service token with narrow scope (e.g., `rpc:com.atproto.repo.getRecord?aud={holdDID}`) -- AppView presents this token to hold as proof - -**Industry usage:** -- `getServiceAuth` appears to be the intended pattern for inter-service auth -- Not widely used yet (ATProto ecosystem is young) -- Most apps use `transition:generic` scope for everything (too broad, not ideal) -- RPC permission scopes are finicky and not well documented - -### Open Questions - -1. **RPC permission format:** Can the `aud` field in RPC permissions support IP addresses? Is this a spec limitation or implementation bug? -2. **Scope granularity:** What's the right balance between `transition:generic` (too broad) and fine-grained RPC scopes (finicky)? -3. **Dynamic discovery + auth:** How should AppView authenticate to arbitrary holds discovered from sailor profiles without pre-registration? -4. **Service token caching:** Should service tokens be cached across multiple requests? Current: 50 second cache, is this optimal? - -## References - -- **Stream.place embedded PDS:** https://streamplace.leaflet.pub/3lut7mgni5s2k/l-quote/6_318-6_554#6 -- **ATProto OAuth spec:** https://atproto.com/specs/oauth -- **ATProto XRPC spec:** https://atproto.com/specs/xrpc -- **ATProto Service Auth:** https://docs.bsky.app/docs/api/com-atproto-server-get-service-auth -- **CID spec:** https://github.com/multiformats/cid -- **OCI Distribution Spec:** https://github.com/opencontainers/distribution-spec diff --git a/docs/HOLD_ENDPOINT_TESTS.md b/docs/HOLD_ENDPOINT_TESTS.md deleted file mode 100644 index edf4da4..0000000 --- a/docs/HOLD_ENDPOINT_TESTS.md +++ /dev/null @@ -1,218 +0,0 @@ -# Hold Service Endpoint Testing Guide - -## Quick Reference - -Your hold service: `http://172.28.0.3:8080` - -Default DID format for local testing: `did:web:172.28.0.3%3A8080` (URL-encoded `did:web:172.28.0.3:8080`) - -## Individual cURL Commands - -### 1. List Repositories -```bash -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.sync.listRepos" | jq . -``` - -**Expected response:** -```json -{ - "repos": [ - { - "did": "did:web:172.28.0.3%3A8080", - "head": "...", - "rev": "..." - } - ] -} -``` - -### 2. Describe Repository -```bash -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.repo.describeRepo?repo=did:web:172.28.0.3%3A8080" | jq . -``` - -**Expected response:** -```json -{ - "did": "did:web:172.28.0.3%3A8080", - "handle": "172.28.0.3:8080", - "didDoc": {...}, - "collections": ["io.atcr.hold.captain", "io.atcr.hold.crew"] -} -``` - -### 3. Get Repository (CAR file) -```bash -# Download entire repo as CAR file -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.sync.getRepo?did=did:web:172.28.0.3%3A8080" -o repo.car - -# Get repo diff since revision -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.sync.getRepo?did=did:web:172.28.0.3%3A8080&since=abc123" -o repo-diff.car -``` - -**Expected response:** Binary CAR (Content Addressable aRchive) file - -### 4. List Captain Records -```bash -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.repo.listRecords?repo=did:web:172.28.0.3%3A8080&collection=io.atcr.hold.captain" | jq . -``` - -**Expected response:** -```json -{ - "records": [ - { - "uri": "at://did:web:172.28.0.3%3A8080/io.atcr.hold.captain/self", - "cid": "...", - "value": { - "$type": "io.atcr.hold.captain", - "allowAllCrew": true, - "public": false, - "createdAt": "2025-10-22T..." - } - } - ] -} -``` - -### 5. List Crew Records -```bash -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.repo.listRecords?repo=did:web:172.28.0.3%3A8080&collection=io.atcr.hold.crew" | jq . -``` - -**Expected response:** -```json -{ - "records": [ - { - "uri": "at://did:web:172.28.0.3%3A8080/io.atcr.hold.crew/{rkey}", - "cid": "...", - "value": { - "$type": "io.atcr.hold.crew", - "did": "did:plc:...", - "permissions": ["blob:read", "blob:write"], - "createdAt": "2025-10-22T..." - } - } - ] -} -``` - -### 6. Get Specific Record -```bash -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.repo.getRecord?repo=did:web:172.28.0.3%3A8080&collection=io.atcr.hold.captain&rkey=self" | jq . -``` - -### 7. Get Blob -```bash -# Replace with actual CID from your hold -curl -s "http://172.28.0.3:8080/xrpc/com.atproto.sync.getBlob?did=did:web:172.28.0.3%3A8080&cid=bafyreiabc123..." | jq . -``` - -**Expected response (for OCI blobs):** -```json -{ - "url": "https://s3.amazonaws.com/bucket/path?presigned-params...", - "expiresAt": "2025-10-22T12:15:00Z" -} -``` - -### 8. Subscribe to Repository Events (WebSocket) - -Using **websocat** (recommended): -```bash -# Install: cargo install websocat -websocat "ws://172.28.0.3:8080/xrpc/com.atproto.sync.subscribeRepos" -``` - -Using **wscat**: -```bash -# Install: npm install -g wscat -wscat -c "ws://172.28.0.3:8080/xrpc/com.atproto.sync.subscribeRepos" -``` - -Using **curl** (HTTP upgrade - may not work with all servers): -```bash -curl -i -N \ - -H "Connection: Upgrade" \ - -H "Upgrade: websocket" \ - -H "Sec-WebSocket-Version: 13" \ - -H "Sec-WebSocket-Key: $(echo -n "test" | base64)" \ - "http://172.28.0.3:8080/xrpc/com.atproto.sync.subscribeRepos" -``` - -**Expected response:** Stream of CBOR-encoded events (commits, identities, handles, etc.) - -## DID Resolution - -### Get DID Document -```bash -curl -s "http://172.28.0.3:8080/.well-known/did.json" | jq . -``` - -**Expected response:** -```json -{ - "@context": ["https://www.w3.org/ns/did/v1"], - "id": "did:web:172.28.0.3%3A8080", - "service": [ - { - "id": "#atproto_pds", - "type": "AtprotoPersonalDataServer", - "serviceEndpoint": "http://172.28.0.3:8080" - } - ] -} -``` - -### Get DID from Handle -```bash -curl -s "http://172.28.0.3:8080/.well-known/atproto-did" -``` - -**Expected response:** Plain text DID -``` -did:web:172.28.0.3%3A8080 -``` - -## Running the Test Script - -```bash -# Default (uses 172.28.0.3:8080) -./test-hold-endpoints.sh - -# Custom hold URL -./test-hold-endpoints.sh "http://localhost:8080" - -# Custom hold URL and DID -./test-hold-endpoints.sh "http://localhost:8080" "did:web:localhost%3A8080" -``` - -## Troubleshooting - -### "Connection refused" -- Ensure hold service is running: `docker ps` or check process -- Verify IP address: `docker inspect | grep IPAddress` - -### "Empty response" or "404 Not Found" -- Check hold service logs for errors -- Verify DID format (use URL-encoded version with `%3A` for `:`) -- Ensure hold has been initialized (should have captain record) - -### WebSocket connection fails -- Install websocat: `cargo install websocat` -- Or install wscat: `npm install -g wscat` -- WebSocket endpoints only work with proper WS clients, not regular curl - -### "No records found" -- Captain record created on hold startup if `HOLD_OWNER` is set -- Crew records created when users call `io.atcr.hold.requestCrew` -- Blobs only exist after pushing container images - -## Next Steps - -After verifying these endpoints work: -1. Test OCI upload endpoints (requires authentication) -2. Push a real container image to create blob data -3. Test blob retrieval with real CIDs -4. Monitor WebSocket events during pushes diff --git a/docs/README_EMBEDDING.md b/docs/README_EMBEDDING.md deleted file mode 100644 index 541641c..0000000 --- a/docs/README_EMBEDDING.md +++ /dev/null @@ -1,183 +0,0 @@ -# README Embedding Feature - -## Overview - -Enhance the repository page (`/r/{handle}/{repository}`) with embedded README content fetched from the source repository, similar to Docker Hub's "Overview" tab. - -## Current State - -The repository page currently shows: -- Repository metadata from OCI annotations -- Short description from `org.opencontainers.image.description` -- External links to source (`org.opencontainers.image.source`) and docs (`org.opencontainers.image.documentation`) -- Tags and manifests lists - -## Proposed Feature - -Automatically fetch and render README.md content from the source repository when available, displaying it in an "Overview" section on the repository page. - -## Implementation Approach - -### 1. Source URL Detection - -Parse `org.opencontainers.image.source` annotation to detect GitHub repositories: -- Pattern: `https://github.com/{owner}/{repo}` -- Extract owner and repo name - -### 2. README Fetching - -Fetch README.md from GitHub via raw content URL: -``` -https://raw.githubusercontent.com/{owner}/{repo}/{branch}/README.md -``` - -Try multiple branch names in order: -1. `main` -2. `master` -3. `develop` - -Fallback if README not found or fetch fails. - -### 3. Markdown Rendering - -Use a Go markdown library to render README content: -- **Option A**: `github.com/gomarkdown/markdown` - Pure Go, fast -- **Option B**: `github.com/yuin/goldmark` - CommonMark compliant, extensible -- **Option C**: Call GitHub's markdown API (requires network call) - -Recommended: `goldmark` for CommonMark compliance and GitHub-flavored markdown support. - -### 4. Caching Strategy - -Cache rendered README to avoid repeated fetches: - -**Option A: In-memory cache** -- Simple, fast -- Lost on restart -- Good for MVP - -**Option B: Database cache** -- Add `readme_html` column to `manifests` table -- Update on new manifest pushes -- Persistent across restarts -- Background job to refresh periodically - -**Option C: Hybrid** -- Cache in database -- Also cache in memory for frequently accessed repos -- TTL-based refresh (e.g., 1 hour) - -### 5. UI Integration - -Add "Overview" section to repository page: -- Show after repository header, before tags/manifests -- Render markdown as HTML -- Apply CSS styling for markdown elements (headings, code blocks, tables, etc.) -- Handle images in README (may need to proxy or allow external images) - -## Implementation Steps - -1. **Add README fetcher** (`pkg/appview/readme/fetcher.go`) - ```go - type Fetcher struct { - httpClient *http.Client - cache Cache - } - - func (f *Fetcher) FetchGitHubReadme(sourceURL string) (string, error) - func (f *Fetcher) RenderMarkdown(content string) (string, error) - ``` - -2. **Update database schema** (optional, for caching) - ```sql - ALTER TABLE manifests ADD COLUMN readme_html TEXT; - ALTER TABLE manifests ADD COLUMN readme_fetched_at TIMESTAMP; - ``` - -3. **Update RepositoryPageHandler** - - Fetch README for repository - - Pass rendered HTML to template - -4. **Update repository.html template** - - Add "Overview" section - - Render HTML safely (use `template.HTML`) - -5. **Add markdown CSS** - - Style headings, code blocks, lists, tables - - Syntax highlighting for code blocks (optional) - -## Security Considerations - -1. **XSS Prevention** - - Sanitize HTML output from markdown renderer - - Use `bluemonday` or similar HTML sanitizer - - Only allow safe HTML elements and attributes - -2. **Rate Limiting** - - Cache aggressively to avoid hitting GitHub rate limits - - Consider GitHub API instead of raw content (requires token but higher limits) - - Handle 429 responses gracefully - -3. **Image Handling** - - README may contain images with relative URLs - - Options: - - Rewrite image URLs to absolute GitHub URLs - - Proxy images through ATCR (caching, security) - - Block external images (simplest, but breaks many READMEs) - -4. **Content Size** - - Limit README size (e.g., 1MB max) - - Truncate very long READMEs with "View on GitHub" link - -## Future Enhancements - -1. **Support other platforms** - - GitLab: `https://gitlab.com/{owner}/{repo}/-/raw/{branch}/README.md` - - Gitea/Forgejo - - Bitbucket - -2. **Custom README upload** - - Allow users to upload custom README via UI - - Store in PDS as `io.atcr.readme` record - - Priority: custom > source repo - -3. **Automatic updates** - - Background job to refresh READMEs periodically - - Webhook support to update on push to source repo - -4. **Syntax highlighting** - - Use highlight.js or similar for code blocks - - Support multiple languages - -## Example Flow - -1. User pushes image with label: `org.opencontainers.image.source=https://github.com/alice/myapp` -2. Manifest stored with source URL annotation -3. User visits `/r/alice/myapp` -4. RepositoryPageHandler: - - Checks cache for README - - If not cached or expired: - - Fetches `https://raw.githubusercontent.com/alice/myapp/main/README.md` - - Renders markdown to HTML - - Sanitizes HTML - - Caches result - - Passes README HTML to template -5. Template renders Overview section with README content - -## Dependencies - -```go -// Markdown rendering -github.com/yuin/goldmark v1.6.0 -github.com/yuin/goldmark-emoji v1.0.2 // GitHub emoji support - -// HTML sanitization -github.com/microcosm-cc/bluemonday v1.0.26 -``` - -## References - -- [OCI Image Spec - Annotations](https://github.com/opencontainers/image-spec/blob/main/annotations.md) -- [Docker Hub Overview tab behavior](https://hub.docker.com/) -- [Goldmark documentation](https://github.com/yuin/goldmark) -- [GitHub raw content URLs](https://raw.githubusercontent.com/) diff --git a/docs/SAILOR.md b/docs/SAILOR.md deleted file mode 100644 index e6daf8a..0000000 --- a/docs/SAILOR.md +++ /dev/null @@ -1,394 +0,0 @@ -# Sailor Profile System - -## Overview - -The sailor profile system allows users to choose which hold (storage service) to use for their container images. This enables: -- **Personal holds** - Use your own S3/Storj/Minio storage -- **Shared holds** - Join a team or community hold -- **Default holds** - Use AppView's default storage (free tier) -- **Transparent infrastructure** - Hold choice doesn't affect image URL - -## Concepts - -**Sailor Profile** (`io.atcr.sailor.profile`): -- Record stored in user's PDS -- Contains `defaultHold` preference (DID or URL) -- Created automatically on first authentication -- Managed via web UI or ATProto client - -**Hold Discovery Priority**: -1. User's sailor profile `defaultHold` (if set) -2. User's own hold records (`io.atcr.hold`) - legacy -3. AppView's `default_hold_did` configuration - -## Sailor Profile Record - -```json -{ - "$type": "io.atcr.sailor.profile", - "defaultHold": "did:web:hold.example.com", - "createdAt": "2025-10-02T12:00:00Z", - "updatedAt": "2025-10-02T12:00:00Z" -} -``` - -**Fields:** -- `defaultHold` (string, optional) - Hold DID or URL (auto-normalized to DID) -- `createdAt` (datetime, required) - Profile creation timestamp -- `updatedAt` (datetime, required) - Last update timestamp - -**Record key:** Always `"self"` (only one profile per user) - -**Collection:** `io.atcr.sailor.profile` - -## Profile Management - -### Automatic Creation - -Profiles are created automatically on first authentication: - -```go -// During OAuth login or Basic Auth token exchange -func (h *Handler) HandleCallback(w http.ResponseWriter, r *http.Request) { - // ... OAuth flow ... - - // Create ATProto client with user's OAuth session - client := atproto.NewClientWithIndigoClient(pdsEndpoint, did, apiClient) - - // Ensure profile exists (creates with AppView's default if not) - err := atproto.EnsureProfile(ctx, client, appViewDefaultHoldDID) -} -``` - -**Behavior:** -- If profile exists → no-op -- If profile doesn't exist → creates with `defaultHold` set to AppView's default -- If AppView has no default configured → creates with empty `defaultHold` - -### Web UI Management - -Users can update their profile via the settings page (`/settings`): - -**View current profile:** -``` -GET /settings -→ Shows current defaultHold value -``` - -**Update defaultHold:** -``` -POST /api/settings/update-hold -Form data: hold_endpoint=did:web:team-hold.fly.dev - -→ Updates sailor profile in user's PDS -→ Returns success confirmation -``` - -**Implementation** (`pkg/appview/handlers/settings.go`): -- Requires OAuth session (user must be logged in) -- Fetches existing profile or creates new one -- Normalizes URLs to DIDs automatically -- Updates `updatedAt` timestamp - -### ATProto Client Management - -Users can also manage their profile using standard ATProto tools: - -**Get profile:** -```bash -atproto get-record \ - --collection io.atcr.sailor.profile \ - --rkey self -``` - -**Update profile:** -```bash -atproto put-record \ - --collection io.atcr.sailor.profile \ - --rkey self \ - --value '{ - "$type": "io.atcr.sailor.profile", - "defaultHold": "did:web:my-hold.example.com", - "updatedAt": "2025-10-20T12:00:00Z" - }' -``` - -**Clear default hold** (opt out): -```bash -atproto put-record \ - --collection io.atcr.sailor.profile \ - --rkey self \ - --value '{ - "$type": "io.atcr.sailor.profile", - "defaultHold": "", - "updatedAt": "2025-10-20T12:00:00Z" - }' -``` - -## URL-to-DID Migration - -The system automatically migrates old URL-based `defaultHold` values to DID format for consistency: - -**Old format (deprecated):** -```json -{ - "defaultHold": "https://hold.example.com" -} -``` - -**New format (preferred):** -```json -{ - "defaultHold": "did:web:hold.example.com" -} -``` - -**Migration behavior:** -- `GetProfile()` detects URL format automatically -- Converts URL → DID transparently (strips protocol, converts to `did:web:`) -- Persists migration to PDS in background goroutine -- Uses locks to prevent duplicate migrations -- Completely transparent to user - -**Why DIDs?** -- **Portable**: DIDs work offline, URLs require DNS -- **Canonical**: One DID per hold, multiple URLs possible -- **Standard**: ATProto uses DIDs for identity - -## Hold Discovery Flow - -When a user pushes an image, AppView discovers which hold to use: - -``` -1. User: docker push atcr.io/alice/myapp:latest - -2. AppView resolves alice → did:plc:alice123 - -3. AppView calls findHoldDID(did, pdsEndpoint): - a. Query alice's PDS for io.atcr.sailor.profile/self - b. If profile.defaultHold is set → use it - c. Else check alice's io.atcr.hold records (legacy) - d. Else use AppView's default_hold_did - -4. Found: alice.profile.defaultHold = "did:web:team-hold.fly.dev" - -5. AppView uses team-hold.fly.dev for blob storage - -6. Manifest stored in alice's PDS includes: - - holdDid: "did:web:team-hold.fly.dev" (for future pulls) - - holdEndpoint: "https://team-hold.fly.dev" (backward compat) -``` - -**Implementation** (`pkg/appview/middleware/registry.go:findHoldDID()`): - -```go -func (nr *NamespaceResolver) findHoldDID(ctx context.Context, did, pdsEndpoint string) string { - client := atproto.NewClient(pdsEndpoint, did, "") - - // 1. Check sailor profile - profile, err := atproto.GetProfile(ctx, client) - if profile != nil && profile.DefaultHold != "" { - return profile.DefaultHold // DID or URL (auto-normalized) - } - - // 2. Check own hold records (legacy) - records, _ := client.ListRecords(ctx, "io.atcr.hold", 10) - for _, record := range records { - // Return first hold's endpoint - if holdRecord.Endpoint != "" { - return atproto.ResolveHoldDIDFromURL(holdRecord.Endpoint) - } - } - - // 3. Use AppView default - return nr.defaultHoldDID -} -``` - -## Use Cases - -### 1. Default Hold (Free Tier) - -User doesn't need to do anything: - -``` -1. User authenticates to atcr.io -2. Profile created with defaultHold = AppView's default -3. User pushes images → blobs go to default hold -``` - -**Profile:** -```json -{ - "defaultHold": "did:web:hold01.atcr.io" -} -``` - -### 2. Join Team Hold - -User joins a shared team hold: - -``` -1. Team admin deploys hold service (did:web:team-hold.fly.dev) -2. Team admin adds user to crew (via hold's PDS) -3. User updates profile: - - Via web UI: /settings → set hold to "did:web:team-hold.fly.dev" - - Or via ATProto client: put-record -4. User pushes images → blobs go to team hold -``` - -**Profile:** -```json -{ - "defaultHold": "did:web:team-hold.fly.dev" -} -``` - -**Benefits:** -- Team pays for storage (not individual users) -- Centralized access control -- Shared bandwidth limits - -### 3. Personal Hold (BYOS) - -User deploys their own hold: - -``` -1. User deploys hold service to Fly.io (did:web:alice-hold.fly.dev) -2. Hold auto-creates captain + crew records on first run -3. User updates profile to use their hold -4. User pushes images → blobs go to personal hold -``` - -**Profile:** -```json -{ - "defaultHold": "did:web:alice-hold.fly.dev" -} -``` - -**Benefits:** -- Full control over storage -- Choose storage provider (S3, Storj, Minio, etc.) -- No quotas/limits (except what you pay for) - -### 4. Opt Out of Defaults - -User wants to use only their own hold records (legacy model): - -```json -{ - "defaultHold": "" -} -``` - -**Behavior:** -- Skips profile's defaultHold (set to empty/null) -- Falls back to `io.atcr.hold` records in user's PDS -- If no hold records found → uses AppView default - -## Architecture Notes - -### Why Sailor Profile? - -**Problem solved:** -- Users can be crew members of multiple holds -- Need explicit way to choose which hold to use -- Want to support both personal and shared holds - -**Without sailor profile:** -``` -Alice is crew of: -- team-hold.fly.dev (team storage) -- community-hold.fly.dev (community storage) - -Which one should AppView use? 🤔 -``` - -**With sailor profile:** -``` -Alice sets profile.defaultHold = "did:web:team-hold.fly.dev" -→ AppView knows to use team hold -→ Alice can change anytime via settings -``` - -### Image Ownership vs Hold Choice - -**Key insight:** Image ownership stays with the user, hold is just infrastructure. - -**URL structure:** `atcr.io//:` -- Owner = Alice (clear ownership) -- Hold = Team storage (infrastructure detail) - -**Analogy:** Like choosing an S3 region -- Your files, your ownership -- Region is just where bits live -- Can move regions without changing ownership - -### Historical Hold References - -Manifests store `holdDid` for immutable blob location tracking: - -```json -{ - "digest": "sha256:abc123", - "holdDid": "did:web:team-hold.fly.dev", - "holdEndpoint": "https://team-hold.fly.dev", - "layers": [...] -} -``` - -**Why store hold in manifest?** -- Pull uses historical reference (not re-discovered) -- Image stays pullable even if user changes defaultHold -- Blobs fetched from where they were originally pushed -- Immutable references (manifests don't change) - -**Hold cache:** -- In-memory cache: `(userDID, repository) → holdDid` -- TTL: 10 minutes (covers typical pull operation) -- Avoids re-querying PDS for every blob - -## Configuration - -### AppView Configuration - -```bash -# Default hold for new users -ATCR_DEFAULT_HOLD_DID=did:web:hold01.atcr.io - -# Test mode: fallback to default if user's hold unreachable -ATCR_TEST_MODE=false -``` - -**Test mode behavior:** -- Checks if user's defaultHold is reachable (HTTP/HTTPS) -- Falls back to AppView default if unreachable -- Useful for local development (prevents errors from unreachable holds) - -### Legacy Support - -**Old hold registration model** (`io.atcr.hold` records in user's PDS): -- Still supported for backward compatibility -- Checked if profile.defaultHold is empty -- New deployments should use sailor profiles instead - -**Migration path:** -- Existing holds continue to work -- Users with `io.atcr.hold` records can set profile.defaultHold -- Profile takes priority over hold records - -## Future Improvements - -1. **Multi-hold support** - Set different holds for different repositories -2. **Hold suggestions** - Recommend holds based on geography/cost -3. **Hold migration tools** - Move blobs between holds -4. **Profile templates** - Pre-configured profiles for teams -5. **Hold analytics** - Show storage usage per hold in UI - -## References - -- [BYOS.md](./BYOS.md) - BYOS deployment and hold management -- [EMBEDDED_PDS.md](./EMBEDDED_PDS.md) - Hold's embedded PDS architecture -- [CREW_ACCESS_CONTROL.md](./CREW_ACCESS_CONTROL.md) - Crew membership and permissions -- [ATProto Lexicon Spec](https://atproto.com/specs/lexicon) diff --git a/docs/TEST_COVERAGE_GAPS.md b/docs/TEST_COVERAGE_GAPS.md new file mode 100644 index 0000000..a0def45 --- /dev/null +++ b/docs/TEST_COVERAGE_GAPS.md @@ -0,0 +1,639 @@ +# Test Coverage Gaps + +**Overall Coverage:** 39.0% (improved from 37.7%, +1.3%) + +This document tracks files in the `pkg/` directory that need test coverage, organized by package. Data is based on actual `coverage.out` analysis. + +**Last Updated:** After adding tests for atproto utilities, handlers improvements, and OAuth browser functionality. + +## Recent Achievements 🎯 + +In this testing session, we achieved: + +1. **pkg/appview/handlers** - 2.1% → 19.7% (**+17.6%** 🎉) + - Significant improvement in web handler coverage + - Better test coverage across handler functions + +2. **pkg/atproto** - 26.1% → 27.8% (**+1.7%**) + - New test files added: + - directory_test.go (NEW) + - endpoints_test.go (NEW) + - utils_test.go (NEW) + - Improved lexicon tests + +3. **pkg/auth/oauth** - 48.3% → 50.7% (**+2.4%**) + - browser_test.go improvements + - Better OAuth flow coverage + +4. **Overall improvement** - 37.7% → 39.0% (**+1.3%**) + - Cumulative improvement from baseline: 31.2% → 39.0% (**+7.8%**) + +**Note:** pkg/appview/db coverage decreased slightly from 44.8% → 41.2% (-3.6%), likely due to additional untested code paths being tracked in existing test files. + +**Next Priority:** Continue with storage blob write operations (proxy_blob_store.go Put/Create/Writer methods) + +--- + +Legend: +- ⭐ **Critical Priority** - Core functionality that must be tested +- 🔴 **High Priority** - Important functionality with security/data implications +- 🟡 **Medium Priority** - Supporting functionality +- 🟢 **Low Priority** - Nice-to-have, less critical features +- ✅ **Good Coverage** - Package has >70% coverage +- 📊 **Partial Coverage** - File has some coverage but needs more +- 🎯 **Recently Improved** - Coverage significantly improved in latest update + +--- + +## Package Coverage Summary + +| Package | Coverage | Status | Priority | Change | +|---------|----------|--------|----------|--------| +| `pkg/hold` | 98.0% | ✅ Excellent | - | - | +| `pkg/s3` | 97.4% | ✅ Excellent | - | - | +| `pkg/appview/licenses` | 93.0% | ✅ Excellent | - | - | +| `pkg/appview` | 81.9% | ✅ Excellent | - | +0.1% | +| `pkg/logging` | 75.0% | ✅ Good | - | - | +| `pkg/auth/token` | 68.8% | 🟡 Good | - | - | +| `pkg/appview/middleware` | 57.8% | 🟡 Good | - | - | +| `pkg/auth` | 55.7% | 🟡 Needs work | Medium | - | +| `pkg/hold/oci` | 51.9% | 🟡 Needs work | Medium | - | +| `pkg/appview/storage` | 51.4% | 🟡 Needs work | **High** | - | +| `pkg/auth/oauth` | 50.7% | 🟡 Needs work | High | 🎯 **+2.4%** | +| `pkg/hold/pds` | 47.2% | 🟡 Needs work | Low | - | +| `pkg/appview/db` | 41.2% | 🟡 Needs work | Medium | 🔴 **-3.6%** | +| `pkg/appview/holdhealth` | 41.0% | 🟡 Needs work | Low | - | +| `pkg/atproto` | 27.8% | 🟡 Needs work | High | 🎯 **+1.7%** | +| `pkg/appview/readme` | 27.2% | 🟡 Needs work | Low | - | +| `pkg/appview/handlers` | 19.7% | 🟡 Needs work | Low | 🎯 **+17.6%** | +| `pkg/appview/jetstream` | 11.6% | 🟡 Needs work | Medium | - | +| `pkg/appview/routes` | 10.4% | 🟡 Needs work | Low | - | + +**⚠️ Notes on Coverage Changes:** + +Several packages show decreased percentages despite improvements. This is due to: +1. **New test files added** - Coverage now tracks previously untested files +2. **Statement weighting** - Large untested functions (like `Repository()` at 0% in middleware) lower overall package percentage +3. **More comprehensive tracking** - Better coverage analysis reveals gaps that were previously invisible + +**Specific file-level improvements (hidden by package averages):** +- `pkg/appview/middleware/auth.go`: 98.8% average (excellent) +- `pkg/appview/middleware/registry.go`: 90.8% average (excellent) +- `pkg/appview/storage/manifest_store.go`: 0% → 85%+ (critical improvement) +- `pkg/atproto/client.go`: 74.8% average (good) +- `pkg/atproto/resolver.go`: 74.5% average (good) + +**Key Insight:** Focus on file-level coverage for critical paths rather than package averages, as new comprehensive testing can paradoxically lower package percentages while improving actual test quality. + +--- + +## Recently Completed ✅ + +### ✅ pkg/appview/storage/manifest_store.go (85%+ coverage) - **COMPLETED** 🎉 + +**Achievement:** Improved from 0% to 85%+ (Critical Priority #1 from previous plan) + +**Well-covered functions:** +- `NewManifestStore()` - 100% ✅ +- `Exists()` - 100% ✅ +- `Get()` - 85.7% ✅ +- `Put()` - 75.5% ✅ +- `Delete()` - 100% ✅ +- `digestToRKey()` - 100% ✅ +- `GetLastFetchedHoldDID()` - 100% ✅ +- `extractConfigLabels()` - 90.0% ✅ +- `resolveDIDToHTTPSEndpoint()` - 100% ✅ + +**Why This Was Critical:** +- Core OCI manifest operations (store/retrieve/delete) +- ATProto record conversion +- Digest-based addressing +- Essential for registry functionality + +**Remaining gaps:** +- `notifyHoldAboutManifest()` - 0% (background notification, less critical) +- `refreshReadmeCache()` - 11.8% (UI feature, lower priority) + +## Critical Priority: Core Registry Functionality + +These components are essential to registry operation and still need coverage. + +### ⭐ pkg/appview/storage (51.4% coverage) - **HIGHEST PRIORITY** + +**Status:** Manifest operations completed ✅, blob write operations remain critical gap + +#### proxy_blob_store.go (Partial coverage) - **HIGHEST PRIORITY** 🎯 + +**Why Critical:** Handles all blob upload/download operations for the registry + +**Well-covered (blob reads and helpers):** +- `NewProxyBlobStore()` - 100% ✅ +- `doAuthenticatedRequest()` - 100% ✅ +- `getPresignedURL()` - 70% ✅ +- `startMultipartUpload()` - 70% ✅ +- `getPartUploadInfo()` - 70% ✅ +- `completeMultipartUpload()` - 75% ✅ +- `abortMultipartUpload()` - 70.6% ✅ +- `Get()` - 68.8% ✅ +- `Open()` - 62.5% ✅ + +**Needs improvement:** +- `Stat()` - 26.3% 📊 +- `checkReadAccess()` - 25.0% 📊 + +**Critical gaps (0% coverage):** +- `Put()` - Main upload entry point (CRITICAL) +- `Create()` - Blob creation (CRITICAL) +- `Delete()` - Blob deletion +- `ServeBlob()` - Blob serving +- `Resume()` - Upload resumption +- `checkWriteAccess()` - Write authorization + +**Writer interface (0% coverage - CRITICAL for uploads):** +- `Write()` - Write data to multipart upload +- `flushPart()` - Flush buffered part +- `ReadFrom()` - io.ReaderFrom implementation +- `Commit()` - Finalize upload +- `Cancel()` - Cancel upload +- `Close()` - Close writer +- `Size()` - Get written size +- `ID()` - Get upload ID +- `StartedAt()` - Get start time +- `Seek()` - Seek in upload + +**Test Scenarios Needed:** +1. Full multipart upload flow: `Put()` → `Create()` → `Write()` → `Commit()` +2. Large blob upload with multiple parts +3. Upload cancellation and cleanup +4. Error handling for failed uploads +5. Upload resumption with `Resume()` +6. Write authorization checks +7. Delete operations + +#### routing_repository.go (Partial coverage) - **HIGH PRIORITY** + +**Current coverage:** +- `Manifests()` - Returns manifest store (mostly tested via manifest_store tests) +- `Blobs()` - 0% coverage (blob routing logic untested) +- `Repository()` - 0% coverage (wrapper method, lower priority) + +**Test Scenarios Needed:** +- Blob routing using cached hold DID (pull scenario) +- Blob routing using discovered hold DID (push scenario) +- Error handling for missing hold +- Hold cache integration + +#### crew.go (11.1% coverage) - **MEDIUM PRIORITY** +**Functions:** +- `EnsureCrewMembership()` - 11.1% +- `requestCrewMembership()` - 0% + +**Test Scenarios Needed:** +- Valid crew member with permissions +- Crew member without required permission +- Non-member access denial +- Crew membership request flow + +#### hold_cache.go (93% coverage) - **EXCELLENT** ✅ + +**Well-covered:** +- `init()` - 80% ✅ +- `GetGlobalHoldCache()` - 100% ✅ +- `Set()` - 100% ✅ +- `Get()` - 100% ✅ +- `Cleanup()` - 100% ✅ + +--- + +## High Priority: Supporting Infrastructure + +### 🔴 pkg/auth/oauth (48.3% coverage, improved from 40.4%) + +OAuth implementation has test files but many functions remain untested. + +#### refresher.go (Partial coverage) + +**Well-covered:** +- `NewRefresher()` - 100% ✅ +- `SetUISessionStore()` - 100% ✅ + +**Critical gaps (0% coverage):** +- `GetSession()` - 0% (CRITICAL - main session retrieval) +- `resumeSession()` - 0% (CRITICAL - session resumption) +- `InvalidateSession()` - 0% +- `GetSessionID()` - 0% + +**Test Scenarios Needed:** +- Session retrieval and caching +- Token refresh flow +- Concurrent refresh handling (per-DID locking) +- Cache expiration +- Error handling for failed refreshes + +#### server.go (Partial coverage) + +**Well-covered:** +- `NewServer()` - 100% ✅ +- `SetRefresher()` - 100% ✅ +- `SetUISessionStore()` - 100% ✅ +- `SetPostAuthCallback()` - 100% ✅ +- `renderRedirectToSettings()` - 80.0% ✅ +- `renderError()` - 83.3% ✅ + +**Critical gaps:** +- `ServeAuthorize()` - 36.8% (needs more coverage) +- `ServeCallback()` - 16.3% (CRITICAL - main OAuth callback handler) + +**Test Scenarios Needed:** +- Authorization flow initiation +- Callback handling with valid code +- Error handling for invalid state/code +- DPoP proof validation +- State parameter validation + +#### interactive.go (41.7% coverage) +**Function:** +- `InteractiveFlowWithCallback()` - 41.7% + +**Test Scenarios Needed:** +- Two-phase callback setup +- Browser interaction flow +- Callback server lifecycle + +#### client.go (Excellent coverage) ✅ + +**Well-covered:** +- `NewApp()` - 100% ✅ +- `NewAppWithScopes()` - 100% ✅ +- `NewClientConfigWithScopes()` - 80.0% ✅ +- `GetConfig()` - 100% ✅ +- `StartAuthFlow()` - 75.0% ✅ +- `ClientIDWithScopes()` - 75.0% ✅ +- `RedirectURI()` - 100% ✅ +- `GetDefaultScopes()` - 100% ✅ +- `ScopesMatch()` - 100% ✅ + +**Improved (from previous 0%):** +- `ProcessCallback()` - Improved coverage +- `ResumeSession()` - Improved coverage +- `GetClientApp()` - Improved coverage +- `Directory()` - Improved coverage (directory_test.go added) + +#### store.go (Good coverage, some gaps) + +**Well-covered:** +- `NewFileStore()` - 100% ✅ +- `GetSession()` - 100% ✅ +- `SaveSession()` - 100% ✅ + +**Gaps:** +- `GetDefaultStorePath()` - 30.0% + +#### browser.go (Improved coverage) 🎯 +**Function:** +- `OpenBrowser()` - Improved coverage (browser_test.go enhanced) + +**Note:** Browser interaction testing improved, though full CI testing remains challenging + +--- + +### 🔴 pkg/appview/db (41.2% coverage, decreased from 44.8%) + +Database layer has test files but many functions remain untested. Coverage decrease likely due to additional code paths being tracked in existing tests. + +#### queries.go (0% coverage for most functions) +**Functions:** +- Repository queries +- Star counting +- Pull counting +- Search queries + +**Test Scenarios Needed:** +- Repository listing with pagination +- Search functionality +- Aggregation queries +- Error handling + +#### session_store.go (0% coverage) +**Functions:** +- Session creation and retrieval +- Session expiration +- Session deletion + +**Test Scenarios Needed:** +- Session lifecycle +- Expiration handling +- Cleanup of expired sessions +- Concurrent session access + +#### device_store.go (📊 Partial coverage) +**Functions:** +- OAuth device flow storage +- Has test file but many functions still at 0% + +**Test Scenarios Needed:** +- User code lookups +- Status updates (pending → approved) +- Expiration handling +- Delete operations + +#### hold_store.go (📊 Partial coverage) +**Needs integration tests for cache invalidation** + +#### oauth_store.go (📊 Partial coverage) +**Uncovered Functions:** +- `GetAuthRequestInfo()` - 0% +- `DeleteAuthRequestInfo()` - 0% +- `SaveAuthRequestInfo()` - 0% + +#### annotations.go (0% coverage) +**Functions:** +- Repository annotations and metadata + +#### readonly.go (0% coverage) +**Functions:** +- Read-only database wrapper + +--- + +## Medium Priority: Supporting Features + +### 🟡 pkg/appview/jetstream (16.7% coverage) + +Event processing for real-time updates. + +#### worker.go (0% coverage) +**Functions:** +- Jetstream event consumption +- Event routing to handlers +- Repository indexing + +#### backfill.go (0% coverage) +**Functions:** +- PDS repository backfilling +- Batch processing + +#### processor.go (📊 Partial coverage) +**Needs more comprehensive testing** + +--- + +### 🟡 pkg/hold/oci (69.9% coverage) + +Multipart upload implementation for hold service. Has good coverage overall but some functions still need tests. + +#### xrpc.go (📊 Partial coverage) +**Functions:** +- Multipart upload XRPC endpoints +- Most functions tested, but edge cases need coverage + +--- + +### 🟡 pkg/hold/pds (57.8% coverage) + +Embedded PDS implementation. Has good test coverage for critical parts, but supporting functions need work. + +#### repomgr.go (📊 Partial coverage) +**Many functions still at 0% coverage** + +#### profile.go (0% coverage) +**Functions:** +- Sailor profile management + +#### layer.go (📊 Partial coverage) +#### auth.go (0% coverage) +#### events.go (📊 Partial coverage) + +--- + +### 🟡 pkg/auth (55.8% coverage) + +#### hold_local.go (0% coverage) +**Functions:** +- Local hold authorization + +#### session.go (0% coverage) +**Functions:** +- Session management + +#### hold_remote.go (📊 Partial coverage) +**Needs more edge case testing** + +--- + +### 🟡 pkg/appview/readme (16.7% coverage) + +README fetching and caching. Less critical but still needs work. + +#### cache.go (0% coverage) +#### fetcher.go (📊 Partial coverage) + +--- + +### 🟡 pkg/appview/routes (33.3% coverage) + +#### routes.go (📊 Partial coverage) +**Needs integration tests for route registration and middleware chains** + +--- + +## Low Priority: Web UI and Supporting Features + +### 🟢 pkg/appview/handlers (19.7% coverage, improved from 2.1%) 🎯 + +Web UI handlers. Less critical than core registry functionality but still important for user experience. + +**Status:** Significant improvement (+17.6%)! Many handlers now have improved test coverage. + +**Improved coverage:** +- Multiple handler functions now have better test coverage +- Common patterns across handlers now tested + +**Files with partial coverage:** +- `common.go` (📊) +- `device.go` (📊) +- `auth.go` (📊) +- `repository.go` (📊) +- `search.go` (📊) +- `settings.go` (📊) +- `user.go` (📊) +- `images.go` (📊) +- `home.go` (📊) +- `install.go` (📊) +- `logout.go` (📊) +- `manifest_health.go` (📊) +- `api.go` (📊) + +**Note:** While individual files may still show gaps, overall handler package coverage has improved significantly. + +--- + +### 🟢 pkg/appview/holdhealth (66.1% coverage) + +Hold health checking. Adequate coverage overall. + +#### worker.go (📊 Partial coverage) +**Could use more edge case testing** + +--- + +### 🟢 pkg/appview/ui.go (0% coverage) + +UI initialization and setup. Low priority. + +--- + +## Recommended Testing Order + +### Phase 1: Critical Infrastructure ✅ **NEARLY COMPLETE** (Target: 45% overall) + +**Completed:** +1. ✅ `pkg/appview/middleware/auth.go` - Authentication (0% → 98.8% avg) +2. ✅ `pkg/appview/middleware/registry.go` - Core routing (0% → 90.8% avg) +3. ✅ `pkg/atproto/client.go` - PDS client (0% → 74.8%) +4. ✅ `pkg/atproto/resolver.go` - Identity resolution (0% → 74.5%) +5. ✅ `pkg/appview/storage/manifest_store.go` - Manifest operations (0% → 85%+) **🎉 COMPLETED** +6. ✅ `pkg/appview/storage/profile.go` - Sailor profiles (NEW → 98%+) **🎉 COMPLETED** + +**Remaining (HIGHEST PRIORITY):** +7. ⭐⭐⭐ `pkg/appview/storage/proxy_blob_store.go` - Blob write operations **CRITICAL** + - `Put()`, `Create()`, Writer interface (0% → 80%+) + - Essential for docker push operations +8. ⭐ `pkg/appview/storage/routing_repository.go` - Blob routing + - `Blobs()` method (0% → 80%+) + +**Current Status:** Overall coverage improved from 37.7% → 39.0% (+1.3%). On track for 45% with Phase 1 completion. + +### Phase 2: Supporting Infrastructure (Target: 50% overall) + +**In Progress:** +9. 🔴 `pkg/appview/db/*` - Database layer (41.2%, needs improvement) + - queries.go, session_store.go, device_store.go +10. 🔴 `pkg/auth/oauth/refresher.go` - Token refresh (Partial → 70%+) + - `GetSession()`, `resumeSession()` (currently 0%) +11. 🔴 `pkg/auth/oauth/server.go` - OAuth endpoints (50.7%, continue improvements) + - `ServeCallback()` at 16.3% needs major improvement +12. 🔴 `pkg/appview/storage/crew.go` - Crew validation (11.1% → 80%+) +13. 🔴 `pkg/auth/*` - Continue auth improvements (55.7% → 70%+) + - hold_remote.go gaps, session.go +14. 🎯 `pkg/atproto/*` - ATProto improvements (27.8%, continue adding tests) + - directory_test.go, endpoints_test.go, utils_test.go added ✅ + +### Phase 3: Event Processing (Target: 55% overall) +15. 🟡 `pkg/appview/jetstream/worker.go` - Event processing (0% → 70%+) +16. 🟡 `pkg/appview/jetstream/backfill.go` - Backfill logic (0% → 70%+) +17. 🟡 `pkg/hold/pds/*` - Fill in gaps in embedded PDS +18. 🟡 `pkg/hold/oci/*` - OCI multipart upload improvements + +### Phase 4: Web UI (Target: 60% overall) +19. 🎯 `pkg/appview/handlers/*` - Web handlers (19.7%, greatly improved from 2.1%) **+17.6%** ✅ + - Continue adding handler tests to reach 50%+ +20. 🟢 `pkg/appview/routes/*` - Route registration (10.4% → 50%+) + +--- + +## Testing Best Practices for This Codebase + +### For Middleware Tests +- Mock HTTP handlers to test middleware wrapping +- Use `httptest.ResponseRecorder` for response inspection +- Test context injection and extraction +- Mock ATProto client for PDS interactions + +### For Storage Tests +- Mock `distribution` interfaces (BlobStore, ManifestService) +- Use in-memory implementations where possible +- Test error propagation from underlying storage +- Mock hold XRPC endpoints + +### For Database Tests +- Use in-memory SQLite (`:memory:`) +- Run migrations in test setup +- Clean up after each test +- Test concurrent operations where relevant + +### For Authorization Tests +- Mock ATProto client for crew lookups +- Test both legacy and new hold models +- Test permission combinations +- Mock service token acquisition + +### For OAuth Tests +- Mock HTTP servers for PDS endpoints +- Test DPoP proof generation/validation +- Test PAR request flow +- Mock browser interaction + +### For ATProto Tests +- Mock HTTP responses for resolver tests +- Test DID document parsing +- Mock XRPC endpoints +- Test authentication flows + +--- + +## Coverage Goals + +**Current:** 39.0% (improved from 37.7%, +1.3%) +**Previous:** 37.7% (improved from 33.5%, +4.2%) +**Total improvement:** 39.0% vs 31.2% baseline = **+7.8%** + +**Top Packages by Coverage:** +- ✅ `pkg/hold`: 98.0% (excellent) +- ✅ `pkg/s3`: 97.4% (excellent) +- ✅ `pkg/appview/licenses`: 93.0% (excellent) +- ✅ `pkg/appview`: 81.8% (excellent) +- ✅ `pkg/logging`: 75.0% (good) + +**Key File-Level Achievements:** +- ✅ `pkg/appview/middleware/auth.go`: 98.8% avg (excellent) +- ✅ `pkg/appview/middleware/registry.go`: 90.8% avg (excellent) +- ✅ `pkg/appview/storage/manifest_store.go`: 85%+ (CRITICAL improvement from 0%) +- ✅ `pkg/appview/storage/profile.go`: 98%+ (new file, excellent) +- ✅ `pkg/atproto/client.go`: 74.8% (good) +- ✅ `pkg/atproto/resolver.go`: 74.5% (good) + +**Packages Needing Work:** +- 🟡 `pkg/auth/token`: 68.8% (good) +- 🟡 `pkg/appview/middleware`: 57.8% (package avg lowered by Repository()) +- 🟡 `pkg/auth`: 55.7% (stable) +- 🟡 `pkg/hold/oci`: 51.9% (needs work) +- 🟡 `pkg/appview/storage`: 51.4% (critical gaps remain) +- 🟡 `pkg/auth/oauth`: 50.7% (improving, was 48.3%) 🎯 **+2.4%** +- 🟡 `pkg/hold/pds`: 47.2% (needs work) +- 🟡 `pkg/appview/db`: 41.2% (decreased from 44.8%, tracking more code paths) 🔴 **-3.6%** +- 🟡 `pkg/atproto`: 27.8% (improving, was 26.1%) 🎯 **+1.7%** +- 🟡 `pkg/appview/handlers`: 19.7% (greatly improved from 2.1%) 🎯 **+17.6%** + +**Short-term Goal (Phase 1 completion):** 45%+ +- ✅ Cover all critical middleware (**COMPLETE**) +- ✅ Cover ATProto client and resolver (**COMPLETE**) +- ✅ Cover storage manifest operations (**COMPLETE** 🎉) +- ⭐ Cover storage blob write operations (**HIGHEST PRIORITY** - Put/Create/Writer) +- ⭐ Cover storage blob routing (**HIGH PRIORITY**) + +**Medium-term Goal (Phase 2):** 50%+ +- Complete remaining storage layer (blob writes) +- Improve database layer coverage (44.8% → 70%+) +- Complete OAuth implementation (refresher.GetSession, server.ServeCallback) +- Add storage crew validation + +**Long-term Goal (Phase 3-4):** 55-60% +- Event processing (jetstream) +- Web UI handlers (currently 2.1%) +- Comprehensive integration tests + +**Realistic Target:** 55-60% (excluding some UI handlers and integration-heavy code) + +**Note:** Package percentages may decrease as new files are added to coverage tracking, but this reflects improved test comprehensiveness, not regression. Focus on file-level coverage for critical paths. + +--- + +## Notes + +- **Test files exist:** Most files in `pkg/` now have corresponding `*_test.go` files, but many functions remain at 0% coverage +- **SQLite vs PostgreSQL:** Current tests use SQLite. For production multi-instance deployments, consider PostgreSQL tests +- **Concurrency:** Many components (cache, token refresher, OAuth) have concurrency concerns that need explicit testing +- **Integration Tests:** Consider adding integration tests that spin up a real PDS + hold service for end-to-end validation +- **Mock Strategy:** Use interfaces (like `atproto.Client`) to enable easy mocking. Consider a mock package in `pkg/testing/` +- **Critical path first:** Focus on middleware and storage layers before web UI, as these are essential for core registry operations diff --git a/pkg/appview/config.go b/pkg/appview/config.go index 1a3e438..ae33d3b 100644 --- a/pkg/appview/config.go +++ b/pkg/appview/config.go @@ -57,6 +57,9 @@ type UIConfig struct { // DatabasePath is the path to the UI SQLite database (from env: ATCR_UI_DATABASE_PATH, default: "/var/lib/atcr/ui.db") DatabasePath string `yaml:"database_path"` + + // SkipDBMigrations controls whether to skip running database migrations (from env: SKIP_DB_MIGRATIONS, default: false) + SkipDBMigrations bool `yaml:"skip_db_migrations"` } // HealthConfig defines health check and cache settings @@ -130,6 +133,7 @@ func LoadConfigFromEnv() (*Config, error) { // UI configuration cfg.UI.Enabled = os.Getenv("ATCR_UI_ENABLED") != "false" cfg.UI.DatabasePath = getEnvOrDefault("ATCR_UI_DATABASE_PATH", "/var/lib/atcr/ui.db") + cfg.UI.SkipDBMigrations = os.Getenv("SKIP_DB_MIGRATIONS") == "true" // Health and cache configuration cfg.Health.CacheTTL = getDurationOrDefault("ATCR_HEALTH_CACHE_TTL", 15*time.Minute) diff --git a/pkg/appview/db/annotations_test.go b/pkg/appview/db/annotations_test.go index 00e97d6..2a73897 100644 --- a/pkg/appview/db/annotations_test.go +++ b/pkg/appview/db/annotations_test.go @@ -21,7 +21,7 @@ func TestAnnotations_Placeholder(t *testing.T) { func setupAnnotationsTestDB(t *testing.T) *sql.DB { t.Helper() // Use file::memory: with cache=shared to ensure all connections share the same in-memory DB - db, err := InitDB("file::memory:?cache=shared") + db, err := InitDB("file::memory:?cache=shared", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } diff --git a/pkg/appview/db/device_store_test.go b/pkg/appview/db/device_store_test.go index 85607f2..c3b3ee2 100644 --- a/pkg/appview/db/device_store_test.go +++ b/pkg/appview/db/device_store_test.go @@ -14,7 +14,7 @@ func setupTestDB(t *testing.T) *DeviceStore { t.Helper() // Use file::memory: with cache=shared to ensure all connections share the same in-memory DB // This prevents race conditions where different connections see different databases - db, err := InitDB("file::memory:?cache=shared") + db, err := InitDB("file::memory:?cache=shared", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } diff --git a/pkg/appview/db/hold_store_test.go b/pkg/appview/db/hold_store_test.go index 6d8c8bb..1e6dedd 100644 --- a/pkg/appview/db/hold_store_test.go +++ b/pkg/appview/db/hold_store_test.go @@ -81,7 +81,7 @@ func TestNullString(t *testing.T) { func setupHoldTestDB(t *testing.T) *sql.DB { t.Helper() // Use file::memory: with cache=shared to ensure all connections share the same in-memory DB - db, err := InitDB("file::memory:?cache=shared") + db, err := InitDB("file::memory:?cache=shared", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } diff --git a/pkg/appview/db/oauth_store_test.go b/pkg/appview/db/oauth_store_test.go index 9ef37d4..5ec2501 100644 --- a/pkg/appview/db/oauth_store_test.go +++ b/pkg/appview/db/oauth_store_test.go @@ -11,7 +11,7 @@ import ( func TestInvalidateSessionsWithMismatchedScopes(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -219,7 +219,7 @@ func TestScopesMatch(t *testing.T) { func TestOAuthStoreSessionLifecycle(t *testing.T) { // Basic test to ensure SaveSession, GetSession, DeleteSession work correctly - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -291,7 +291,7 @@ func TestOAuthStoreSessionLifecycle(t *testing.T) { } func TestCleanupOldSessions(t *testing.T) { - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } diff --git a/pkg/appview/db/queries_test.go b/pkg/appview/db/queries_test.go index f1ad308..3cbb7bd 100644 --- a/pkg/appview/db/queries_test.go +++ b/pkg/appview/db/queries_test.go @@ -7,7 +7,7 @@ import ( func TestGetRepositoryMetadata(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -143,7 +143,7 @@ func TestGetRepositoryMetadata(t *testing.T) { func TestInsertManifest(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -320,7 +320,7 @@ func TestInsertManifest(t *testing.T) { func TestUserManagement(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -432,7 +432,7 @@ func TestUserManagement(t *testing.T) { func TestManifestOperations(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -609,7 +609,7 @@ func TestManifestOperations(t *testing.T) { func TestIsManifestTagged(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -675,7 +675,7 @@ func TestIsManifestTagged(t *testing.T) { func TestTagOperations(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -838,7 +838,7 @@ func TestTagOperations(t *testing.T) { func TestGetTagsWithPlatforms(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } @@ -980,7 +980,7 @@ func TestGetTagsWithPlatforms(t *testing.T) { func TestUpdateUserHandle(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } diff --git a/pkg/appview/db/readonly.go b/pkg/appview/db/readonly.go index 3cae1f7..7e208dc 100644 --- a/pkg/appview/db/readonly.go +++ b/pkg/appview/db/readonly.go @@ -57,7 +57,7 @@ func init() { // InitializeDatabase initializes the SQLite database and session store // Returns: (read-write DB, read-only DB, session store) -func InitializeDatabase(uiEnabled bool, dbPath string) (*sql.DB, *sql.DB, *SessionStore) { +func InitializeDatabase(uiEnabled bool, dbPath string, skipMigrations bool) (*sql.DB, *sql.DB, *SessionStore) { if !uiEnabled { return nil, nil, nil } @@ -70,7 +70,7 @@ func InitializeDatabase(uiEnabled bool, dbPath string) (*sql.DB, *sql.DB, *Sessi } // Initialize read-write database (for writes and auth operations) - database, err := InitDB(dbPath) + database, err := InitDB(dbPath, skipMigrations) if err != nil { slog.Warn("Failed to initialize UI database", "error", err) return nil, nil, nil diff --git a/pkg/appview/db/readonly_test.go b/pkg/appview/db/readonly_test.go index cbb43e1..2282e97 100644 --- a/pkg/appview/db/readonly_test.go +++ b/pkg/appview/db/readonly_test.go @@ -19,7 +19,7 @@ func TestAuthorizerBlocksSensitiveTables(t *testing.T) { defer os.Unsetenv("ATCR_UI_DATABASE_PATH") // Initialize database (creates schema) - database, err := InitDB(dbPath) + database, err := InitDB(dbPath, true) if err != nil { t.Fatalf("Failed to initialize database: %v", err) } diff --git a/pkg/appview/db/schema.go b/pkg/appview/db/schema.go index 1a44026..1e1ed89 100644 --- a/pkg/appview/db/schema.go +++ b/pkg/appview/db/schema.go @@ -26,7 +26,7 @@ var migrationsFS embed.FS var schemaSQL string // InitDB initializes the SQLite database with the schema -func InitDB(path string) (*sql.DB, error) { +func InitDB(path string, skipMigrations bool) (*sql.DB, error) { db, err := sql.Open("sqlite3", path) if err != nil { return nil, err @@ -42,9 +42,11 @@ func InitDB(path string) (*sql.DB, error) { return nil, err } - // Run migrations - if err := runMigrations(db); err != nil { - return nil, err + // Run migrations unless skipped + if !skipMigrations { + if err := runMigrations(db); err != nil { + return nil, err + } } return db, nil diff --git a/pkg/appview/db/session_store_test.go b/pkg/appview/db/session_store_test.go index 8b57a96..2d2ba89 100644 --- a/pkg/appview/db/session_store_test.go +++ b/pkg/appview/db/session_store_test.go @@ -13,7 +13,7 @@ import ( func setupSessionTestDB(t *testing.T) *SessionStore { t.Helper() // Use file::memory: with cache=shared to ensure all connections share the same in-memory DB - db, err := InitDB("file::memory:?cache=shared") + db, err := InitDB("file::memory:?cache=shared", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } diff --git a/pkg/appview/db/tag_delete_test.go b/pkg/appview/db/tag_delete_test.go index 28892c0..88a6317 100644 --- a/pkg/appview/db/tag_delete_test.go +++ b/pkg/appview/db/tag_delete_test.go @@ -11,7 +11,7 @@ import ( // This simulates what Jetstream does: encode repo/tag to rkey, then decode and delete func TestTagDeleteRoundTrip(t *testing.T) { // Create in-memory test database - db, err := InitDB(":memory:") + db, err := InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to init database: %v", err) } diff --git a/pkg/appview/handlers/api_test.go b/pkg/appview/handlers/api_test.go deleted file mode 100644 index 0737881..0000000 --- a/pkg/appview/handlers/api_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestStarRepositoryHandler_Exists(t *testing.T) { - handler := &StarRepositoryHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add API endpoint tests diff --git a/pkg/appview/handlers/auth_test.go b/pkg/appview/handlers/auth_test.go deleted file mode 100644 index 00691e4..0000000 --- a/pkg/appview/handlers/auth_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestLoginHandler_Exists(t *testing.T) { - handler := &LoginHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add template rendering tests diff --git a/pkg/appview/handlers/device_test.go b/pkg/appview/handlers/device_test.go index de7283b..87a9f70 100644 --- a/pkg/appview/handlers/device_test.go +++ b/pkg/appview/handlers/device_test.go @@ -1,80 +1,101 @@ package handlers import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "net/http" "net/http/httptest" + "strings" "testing" + "time" + + "atcr.io/pkg/appview/db" + "github.com/go-chi/chi/v5" + _ "github.com/mattn/go-sqlite3" ) +// setupTestDB creates an in-memory SQLite database with full schema for testing +func setupTestDB(t *testing.T) *sql.DB { + database, err := db.InitDB(":memory:", true) + if err != nil { + t.Fatalf("Failed to initialize test database: %v", err) + } + return database +} + +// Test getClientIP function (existing test, expanded) func TestGetClientIP(t *testing.T) { tests := []struct { - name string - remoteAddr string - xForwardedFor string - xRealIP string - expectedIP string + name string + remoteAddr string + xForwardedFor string + xRealIP string + expectedIP string }{ { - name: "X-Forwarded-For single IP", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "10.0.0.1", - xRealIP: "", - expectedIP: "10.0.0.1", + name: "X-Forwarded-For single IP", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "10.0.0.1", + xRealIP: "", + expectedIP: "10.0.0.1", }, { - name: "X-Forwarded-For multiple IPs", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "10.0.0.1, 10.0.0.2, 10.0.0.3", - xRealIP: "", - expectedIP: "10.0.0.1", + name: "X-Forwarded-For multiple IPs", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "10.0.0.1, 10.0.0.2, 10.0.0.3", + xRealIP: "", + expectedIP: "10.0.0.1", }, { - name: "X-Forwarded-For with whitespace", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: " 10.0.0.1 ", - xRealIP: "", - expectedIP: "10.0.0.1", + name: "X-Forwarded-For with whitespace", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: " 10.0.0.1 ", + xRealIP: "", + expectedIP: "10.0.0.1", }, { - name: "X-Real-IP when no X-Forwarded-For", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "", - xRealIP: "10.0.0.2", - expectedIP: "10.0.0.2", + name: "X-Real-IP when no X-Forwarded-For", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "", + xRealIP: "10.0.0.2", + expectedIP: "10.0.0.2", }, { - name: "X-Forwarded-For takes priority over X-Real-IP", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "10.0.0.1", - xRealIP: "10.0.0.2", - expectedIP: "10.0.0.1", + name: "X-Forwarded-For takes priority over X-Real-IP", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "10.0.0.1", + xRealIP: "10.0.0.2", + expectedIP: "10.0.0.1", }, { - name: "RemoteAddr fallback with port", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "", - xRealIP: "", - expectedIP: "192.168.1.1", + name: "RemoteAddr fallback with port", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "", + xRealIP: "", + expectedIP: "192.168.1.1", }, { - name: "RemoteAddr fallback without port", - remoteAddr: "192.168.1.1", - xForwardedFor: "", - xRealIP: "", - expectedIP: "192.168.1.1", + name: "RemoteAddr fallback without port", + remoteAddr: "192.168.1.1", + xForwardedFor: "", + xRealIP: "", + expectedIP: "192.168.1.1", }, { - name: "IPv6 RemoteAddr", - remoteAddr: "[::1]:1234", - xForwardedFor: "", - xRealIP: "", - expectedIP: "[", + name: "IPv6 RemoteAddr", + remoteAddr: "[::1]:1234", + xForwardedFor: "", + xRealIP: "", + expectedIP: "[", }, { - name: "IPv6 in X-Forwarded-For", - remoteAddr: "192.168.1.1:1234", - xForwardedFor: "2001:db8::1", - xRealIP: "", - expectedIP: "2001:db8::1", + name: "IPv6 in X-Forwarded-For", + remoteAddr: "192.168.1.1:1234", + xForwardedFor: "2001:db8::1", + xRealIP: "", + expectedIP: "2001:db8::1", }, } @@ -99,4 +120,584 @@ func TestGetClientIP(t *testing.T) { } } -// TODO: Add device approval flow tests +func TestDeviceCodeHandler_Success(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceCodeHandler{ + Store: store, + AppViewBaseURL: "http://localhost:5000", + } + + reqBody := DeviceCodeRequest{ + DeviceName: "My Test Device", + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/auth/device/code", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response DeviceCodeResponse + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response.DeviceCode == "" { + t.Error("Expected device_code to be set") + } + if response.UserCode == "" { + t.Error("Expected user_code to be set") + } + if !strings.HasPrefix(response.VerificationURI, "http://localhost:5000") { + t.Errorf("Expected verification_uri to start with base URL, got %s", response.VerificationURI) + } + if response.ExpiresIn != 600 { + t.Errorf("Expected expires_in to be 600, got %d", response.ExpiresIn) + } + if response.Interval != 5 { + t.Errorf("Expected interval to be 5, got %d", response.Interval) + } +} + +func TestDeviceCodeHandler_DefaultDeviceName(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceCodeHandler{ + Store: store, + AppViewBaseURL: "http://localhost:5000", + } + + // Empty device name should get default + reqBody := DeviceCodeRequest{ + DeviceName: "", + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/auth/device/code", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response DeviceCodeResponse + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response.UserCode == "" { + t.Error("Expected user_code to be set even with default device name") + } +} + +func TestDeviceCodeHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceCodeHandler{ + Store: store, + AppViewBaseURL: "http://localhost:5000", + } + + req := httptest.NewRequest("GET", "/auth/device/code", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestDeviceTokenHandler_AuthorizationPending(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceTokenHandler{ + Store: store, + } + + // Create a pending authorization + pending, err := store.CreatePendingAuth("Test Device", "127.0.0.1", "TestAgent/1.0") + if err != nil { + t.Fatalf("Failed to create pending auth: %v", err) + } + + // Poll before approval + reqBody := DeviceTokenRequest{ + DeviceCode: pending.DeviceCode, + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/auth/device/token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response DeviceTokenResponse + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response.Error != "authorization_pending" { + t.Errorf("Expected error 'authorization_pending', got %s", response.Error) + } +} + +func TestDeviceTokenHandler_ExpiredToken(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceTokenHandler{ + Store: store, + } + + // Try to poll with invalid device code + reqBody := DeviceTokenRequest{ + DeviceCode: "invalid_code_12345", + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/auth/device/token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response DeviceTokenResponse + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response.Error != "expired_token" { + t.Errorf("Expected error 'expired_token', got %s", response.Error) + } +} + +func TestDeviceTokenHandler_Approved(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceTokenHandler{ + Store: store, + } + + // Create a pending authorization + pending, err := store.CreatePendingAuth("Test Device", "127.0.0.1", "TestAgent/1.0") + if err != nil { + t.Fatalf("Failed to create pending auth: %v", err) + } + + // Create user first (required for foreign key) + _, err = database.Exec(` + INSERT INTO users (did, handle, pds_endpoint, last_seen) + VALUES (?, ?, ?, ?) + `, "did:plc:test123", "test.bsky.social", "https://bsky.social", time.Now()) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + // Approve it + _, err = store.ApprovePending(pending.UserCode, "did:plc:test123", "test.bsky.social") + if err != nil { + t.Fatalf("Failed to approve pending: %v", err) + } + + // Poll after approval + reqBody := DeviceTokenRequest{ + DeviceCode: pending.DeviceCode, + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/auth/device/token", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response DeviceTokenResponse + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response.Error != "" { + t.Errorf("Expected no error, got %s", response.Error) + } + if response.DeviceSecret == "" { + t.Error("Expected device_secret to be set") + } + if response.DID != "did:plc:test123" { + t.Errorf("Expected DID 'did:plc:test123', got %s", response.DID) + } +} + +func TestDeviceTokenHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + handler := &DeviceTokenHandler{ + Store: store, + } + + req := httptest.NewRequest("GET", "/auth/device/token", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestDeviceApprovalPageHandler_NotLoggedIn(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &DeviceApprovalPageHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/device?user_code=ABC123", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + // Should redirect to login + if rr.Code != http.StatusFound { + t.Errorf("Expected status %d, got %d", http.StatusFound, rr.Code) + } + + location := rr.Header().Get("Location") + if !strings.Contains(location, "/auth/oauth/login") { + t.Errorf("Expected redirect to login, got %s", location) + } +} + +func TestDeviceApprovalPageHandler_MissingUserCode(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + // Create user first (required for foreign key) + _, err := database.Exec(` + INSERT INTO users (did, handle, pds_endpoint, last_seen) + VALUES (?, ?, ?, ?) + `, "did:plc:test123", "test.bsky.social", "https://bsky.social", time.Now()) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + // Create a session + sessionID, _ := sessionStore.Create("did:plc:test123", "test.bsky.social", "https://pds.example.com", 24*time.Hour) + + handler := &DeviceApprovalPageHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/device", nil) // No user_code parameter + req.AddCookie(&http.Cookie{ + Name: "atcr_session", + Value: sessionID, + }) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusBadRequest { + t.Errorf("Expected status %d, got %d", http.StatusBadRequest, rr.Code) + } +} + +func TestDeviceApprovalPageHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &DeviceApprovalPageHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("POST", "/device?user_code=ABC123", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestDeviceApproveHandler_Unauthorized(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &DeviceApproveHandler{ + Store: store, + SessionStore: sessionStore, + } + + reqBody := DeviceApproveRequest{ + UserCode: "ABC123", + Approve: true, + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/device/approve", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d, got %d", http.StatusUnauthorized, rr.Code) + } +} + +func TestDeviceApproveHandler_Deny(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + // Create user first (required for foreign key) + _, err := database.Exec(` + INSERT INTO users (did, handle, pds_endpoint, last_seen) + VALUES (?, ?, ?, ?) + `, "did:plc:test123", "test.bsky.social", "https://bsky.social", time.Now()) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + // Create a session + sessionID, _ := sessionStore.Create("did:plc:test123", "test.bsky.social", "https://pds.example.com", 24*time.Hour) + + handler := &DeviceApproveHandler{ + Store: store, + SessionStore: sessionStore, + } + + reqBody := DeviceApproveRequest{ + UserCode: "ABC123", + Approve: false, + } + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/device/approve", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.AddCookie(&http.Cookie{ + Name: "atcr_session", + Value: sessionID, + }) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var response map[string]string + if err := json.NewDecoder(rr.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response["status"] != "denied" { + t.Errorf("Expected status 'denied', got %s", response["status"]) + } +} + +func TestDeviceApproveHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &DeviceApproveHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/device/approve", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestListDevicesHandler_Unauthorized(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &ListDevicesHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/api/devices", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d, got %d", http.StatusUnauthorized, rr.Code) + } +} + +func TestListDevicesHandler_Success(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + // Create user first (required for foreign key) + _, err := database.Exec(` + INSERT INTO users (did, handle, pds_endpoint, last_seen) + VALUES (?, ?, ?, ?) + `, "did:plc:test123", "test.bsky.social", "https://bsky.social", time.Now()) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + // Create a session + sessionID, _ := sessionStore.Create("did:plc:test123", "test.bsky.social", "https://pds.example.com", 24*time.Hour) + + // Create some devices + pending, _ := store.CreatePendingAuth("Device 1", "127.0.0.1", "TestAgent/1.0") + store.ApprovePending(pending.UserCode, "did:plc:test123", "test.bsky.social") + + handler := &ListDevicesHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/api/devices", nil) + req.AddCookie(&http.Cookie{ + Name: "atcr_session", + Value: sessionID, + }) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d", http.StatusOK, rr.Code) + } + + var devices []db.Device + if err := json.NewDecoder(rr.Body).Decode(&devices); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if len(devices) != 1 { + t.Errorf("Expected 1 device, got %d", len(devices)) + } +} + +func TestListDevicesHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &ListDevicesHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("POST", "/api/devices", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestRevokeDeviceHandler_Unauthorized(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &RevokeDeviceHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("DELETE", "/api/devices/device123", nil) + + // Add chi URL parameter + rctx := chi.NewRouteContext() + rctx.URLParams.Add("id", "device123") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d, got %d", http.StatusUnauthorized, rr.Code) + } +} + +func TestRevokeDeviceHandler_MethodNotAllowed(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + store := db.NewDeviceStore(database) + sessionStore := db.NewSessionStore(database) + + handler := &RevokeDeviceHandler{ + Store: store, + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/api/devices/device123", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} diff --git a/pkg/appview/handlers/home_test.go b/pkg/appview/handlers/home_test.go deleted file mode 100644 index 8759993..0000000 --- a/pkg/appview/handlers/home_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestHomeHandler_Exists(t *testing.T) { - handler := &HomeHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add comprehensive handler tests diff --git a/pkg/appview/handlers/images_test.go b/pkg/appview/handlers/images_test.go index 65475b9..227141e 100644 --- a/pkg/appview/handlers/images_test.go +++ b/pkg/appview/handlers/images_test.go @@ -1,14 +1,68 @@ package handlers import ( + "context" + "net/http" + "net/http/httptest" "testing" + + "github.com/go-chi/chi/v5" ) -func TestDeleteTagHandler_Exists(t *testing.T) { - handler := &DeleteTagHandler{} - if handler == nil { - t.Error("Expected non-nil handler") +func TestDeleteTagHandler_Unauthorized(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + handler := &DeleteTagHandler{ + DB: database, + } + + req := httptest.NewRequest("DELETE", "/alice/myapp/tags/latest", nil) + + // Add chi URL parameters + rctx := chi.NewRouteContext() + rctx.URLParams.Add("handle", "alice") + rctx.URLParams.Add("repository", "myapp") + rctx.URLParams.Add("tag", "latest") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + // Should return unauthorized without user in context + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d, got %d", http.StatusUnauthorized, rr.Code) } } -// TODO: Add image listing tests +func TestDeleteManifestHandler_Unauthorized(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + handler := &DeleteManifestHandler{ + DB: database, + } + + req := httptest.NewRequest("DELETE", "/alice/myapp/manifests/sha256:abc123", nil) + + // Add chi URL parameters + rctx := chi.NewRouteContext() + rctx.URLParams.Add("handle", "alice") + rctx.URLParams.Add("repository", "myapp") + rctx.URLParams.Add("digest", "sha256:abc123") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + // Should return unauthorized without user in context + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected status %d, got %d", http.StatusUnauthorized, rr.Code) + } +} + +// TODO: Add comprehensive tests with authentication +// - Test tag deletion with proper auth +// - Test manifest deletion with proper auth +// - Test deletion of non-existent tags +// - Test unauthorized deletion attempts (wrong user) diff --git a/pkg/appview/handlers/install_test.go b/pkg/appview/handlers/install_test.go deleted file mode 100644 index 1e4c3a8..0000000 --- a/pkg/appview/handlers/install_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestInstallHandler_Exists(t *testing.T) { - handler := &InstallHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add installation instructions tests diff --git a/pkg/appview/handlers/logout_test.go b/pkg/appview/handlers/logout_test.go index c53ddfe..4aee471 100644 --- a/pkg/appview/handlers/logout_test.go +++ b/pkg/appview/handlers/logout_test.go @@ -1,14 +1,97 @@ package handlers import ( + "net/http" + "net/http/httptest" "testing" + "time" + + "atcr.io/pkg/appview/db" ) -func TestLogoutHandler_Exists(t *testing.T) { - handler := &LogoutHandler{} - if handler == nil { - t.Error("Expected non-nil handler") +func TestLogoutHandler_NoSession(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + sessionStore := db.NewSessionStore(database) + + handler := &LogoutHandler{ + SessionStore: sessionStore, + } + + req := httptest.NewRequest("GET", "/auth/logout", nil) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + // Should redirect even with no session + if rr.Code != http.StatusFound { + t.Errorf("Expected status %d, got %d", http.StatusFound, rr.Code) + } + + location := rr.Header().Get("Location") + if location != "/" { + t.Errorf("Expected redirect to /, got %s", location) } } -// TODO: Add cookie clearing tests +func TestLogoutHandler_WithSession(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + sessionStore := db.NewSessionStore(database) + + // Create a user first (required for foreign key) + _, err := database.Exec(` + INSERT INTO users (did, handle, pds_endpoint, last_seen) + VALUES (?, ?, ?, ?) + `, "did:plc:test123", "test.bsky.social", "https://bsky.social", time.Now()) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + // Create a session + sessionID, err := sessionStore.Create("did:plc:test123", "test.bsky.social", "https://bsky.social", 24*time.Hour) + if err != nil { + t.Fatalf("Failed to create session: %v", err) + } + + handler := &LogoutHandler{ + SessionStore: sessionStore, + OAuthStore: db.NewOAuthStore(database), + } + + req := httptest.NewRequest("GET", "/auth/logout", nil) + req.AddCookie(&http.Cookie{ + Name: "atcr_session", + Value: sessionID, + }) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + // Should redirect + if rr.Code != http.StatusFound { + t.Errorf("Expected status %d, got %d", http.StatusFound, rr.Code) + } + + // Should clear cookie + cookies := rr.Result().Cookies() + found := false + for _, cookie := range cookies { + if cookie.Name == "atcr_session" { + found = true + if cookie.MaxAge != -1 { + t.Errorf("Expected cookie MaxAge=-1, got %d", cookie.MaxAge) + } + } + } + if !found { + t.Error("Expected atcr_session cookie to be cleared") + } + + // Session should be deleted + _, exists := sessionStore.Get(sessionID) + if exists { + t.Error("Expected session to be deleted") + } +} diff --git a/pkg/appview/handlers/manifest_health_test.go b/pkg/appview/handlers/manifest_health_test.go deleted file mode 100644 index 15773ff..0000000 --- a/pkg/appview/handlers/manifest_health_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestManifestHealthHandler_Exists(t *testing.T) { - handler := &ManifestHealthHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add manifest health check tests diff --git a/pkg/appview/handlers/repository_test.go b/pkg/appview/handlers/repository_test.go deleted file mode 100644 index 6e4533f..0000000 --- a/pkg/appview/handlers/repository_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestRepositoryPageHandler_Exists(t *testing.T) { - handler := &RepositoryPageHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add comprehensive tests with mocked database diff --git a/pkg/appview/handlers/search_test.go b/pkg/appview/handlers/search_test.go deleted file mode 100644 index 5421bee..0000000 --- a/pkg/appview/handlers/search_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestSearchHandler_Exists(t *testing.T) { - handler := &SearchHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add query parsing tests diff --git a/pkg/appview/handlers/settings_test.go b/pkg/appview/handlers/settings_test.go deleted file mode 100644 index 90258bb..0000000 --- a/pkg/appview/handlers/settings_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestSettingsHandler_Exists(t *testing.T) { - handler := &SettingsHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add settings page tests diff --git a/pkg/appview/handlers/user_test.go b/pkg/appview/handlers/user_test.go deleted file mode 100644 index a1cd2c9..0000000 --- a/pkg/appview/handlers/user_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package handlers - -import ( - "testing" -) - -func TestUserPageHandler_Exists(t *testing.T) { - handler := &UserPageHandler{} - if handler == nil { - t.Error("Expected non-nil handler") - } -} - -// TODO: Add user profile tests diff --git a/pkg/appview/middleware/auth_test.go b/pkg/appview/middleware/auth_test.go index 46680d2..09a6802 100644 --- a/pkg/appview/middleware/auth_test.go +++ b/pkg/appview/middleware/auth_test.go @@ -26,7 +26,7 @@ func TestGetUser_NoContext(t *testing.T) { // setupTestDB creates an in-memory SQLite database for testing func setupTestDB(t *testing.T) *sql.DB { - database, err := db.InitDB(":memory:") + database, err := db.InitDB(":memory:", true) require.NoError(t, err) t.Cleanup(func() { @@ -307,7 +307,7 @@ func TestOptionalAuth_InvalidSession(t *testing.T) { func TestMiddleware_ConcurrentAccess(t *testing.T) { // Use a shared in-memory database for concurrent access // (SQLite's default :memory: creates separate DBs per connection) - database, err := db.InitDB("file::memory:?cache=shared") + database, err := db.InitDB("file::memory:?cache=shared", true) require.NoError(t, err) t.Cleanup(func() { database.Close() diff --git a/pkg/atproto/directory_test.go b/pkg/atproto/directory_test.go new file mode 100644 index 0000000..1e81d55 --- /dev/null +++ b/pkg/atproto/directory_test.go @@ -0,0 +1,151 @@ +package atproto + +import ( + "sync" + "testing" +) + +func TestGetDirectorySingleton(t *testing.T) { + t.Run("returns non-nil directory", func(t *testing.T) { + dir := GetDirectory() + if dir == nil { + t.Fatal("GetDirectory() returned nil") + } + }) + + t.Run("singleton behavior - same instance", func(t *testing.T) { + // Get directory twice + dir1 := GetDirectory() + dir2 := GetDirectory() + + // They should be the exact same instance (same pointer) + if dir1 != dir2 { + t.Error("GetDirectory() returned different instances, expected singleton") + } + }) +} + +func TestGetDirectoryConcurrency(t *testing.T) { + t.Run("concurrent access is thread-safe", func(t *testing.T) { + const numGoroutines = 100 + var wg sync.WaitGroup + wg.Add(numGoroutines) + + // Channel to collect all directory instances + instances := make(chan interface{}, numGoroutines) + + // Launch many goroutines concurrently accessing GetDirectory + for i := 0; i < numGoroutines; i++ { + go func() { + defer wg.Done() + dir := GetDirectory() + instances <- dir + }() + } + + // Wait for all goroutines to complete + wg.Wait() + close(instances) + + // Collect all instances + var dirs []interface{} + for dir := range instances { + dirs = append(dirs, dir) + } + + // Verify we got the expected number of results + if len(dirs) != numGoroutines { + t.Fatalf("Expected %d directory instances, got %d", numGoroutines, len(dirs)) + } + + // All instances should be identical (singleton) + firstDir := dirs[0] + for i, dir := range dirs { + if dir != firstDir { + t.Errorf("Directory instance %d differs from first instance", i) + } + } + }) + +} + +func TestGetDirectorySequential(t *testing.T) { + t.Run("multiple calls in sequence", func(t *testing.T) { + // Get directory multiple times in sequence + dirs := make([]interface{}, 10) + for i := 0; i < 10; i++ { + dirs[i] = GetDirectory() + } + + // All should be the same instance + for i := 1; i < len(dirs); i++ { + if dirs[i] != dirs[0] { + t.Errorf("Call %d returned different instance than first call", i) + } + } + }) +} + +// TestGetDirectoryInterface verifies the directory is properly initialized +func TestGetDirectoryInterface(t *testing.T) { + // Verify the directory instance works as expected + dir := GetDirectory() + + // Verify directory is not nil + if dir == nil { + t.Fatal("Directory should not be nil") + } + + // Verify it's the indigo Directory interface type + // We can't easily introspect the methods without importing indigo's types, + // but we can verify the instance is usable by checking it's not nil + // and that it's the same as subsequent calls (already tested above) + + // Additional verification: the directory should be the same across calls + dir2 := GetDirectory() + if dir != dir2 { + t.Error("Directory instances differ, singleton pattern broken") + } +} + +// TestGetDirectoryRaceConditions specifically tests race conditions during initialization +func TestGetDirectoryRaceConditions(t *testing.T) { + // This test would ideally reset the singleton, but since we can't do that + // safely, we instead verify that even if GetDirectory is called concurrently + // before initialization completes, it still works correctly. + // + // The sync.Once ensures this is safe, so calling GetDirectory from multiple + // goroutines simultaneously should still result in exactly one initialization + // and all goroutines getting the same instance. + + const numGoroutines = 50 + var wg sync.WaitGroup + wg.Add(numGoroutines) + + instances := make([]interface{}, numGoroutines) + var mu sync.Mutex + + // Simulate many goroutines trying to get the directory simultaneously + for i := 0; i < numGoroutines; i++ { + go func(idx int) { + defer wg.Done() + dir := GetDirectory() + mu.Lock() + instances[idx] = dir + mu.Unlock() + }(i) + } + + wg.Wait() + + // Verify all instances are identical + firstDir := instances[0] + for i, dir := range instances { + if dir == nil { + t.Errorf("Instance %d is nil", i) + } + if dir != firstDir { + t.Errorf("Instance %d differs from first instance", i) + } + } +} diff --git a/pkg/atproto/endpoints_test.go b/pkg/atproto/endpoints_test.go new file mode 100644 index 0000000..05a6a26 --- /dev/null +++ b/pkg/atproto/endpoints_test.go @@ -0,0 +1,262 @@ +package atproto + +import ( + "strings" + "testing" +) + +// TestEndpointsFormat validates that all endpoint constants follow the XRPC convention +func TestEndpointsFormat(t *testing.T) { + tests := []struct { + name string + endpoint string + prefix string // Expected namespace prefix (e.g., "io.atcr" or "com.atproto") + }{ + // Hold service multipart upload endpoints + {"HoldInitiateUpload", HoldInitiateUpload, "io.atcr.hold"}, + {"HoldGetPartUploadURL", HoldGetPartUploadURL, "io.atcr.hold"}, + {"HoldUploadPart", HoldUploadPart, "io.atcr.hold"}, + {"HoldCompleteUpload", HoldCompleteUpload, "io.atcr.hold"}, + {"HoldAbortUpload", HoldAbortUpload, "io.atcr.hold"}, + {"HoldNotifyManifest", HoldNotifyManifest, "io.atcr.hold"}, + + // Hold service crew management endpoints + {"HoldRequestCrew", HoldRequestCrew, "io.atcr.hold"}, + + // ATProto sync endpoints + {"SyncGetBlob", SyncGetBlob, "com.atproto.sync"}, + {"SyncGetRepo", SyncGetRepo, "com.atproto.sync"}, + {"SyncGetRecord", SyncGetRecord, "com.atproto.sync"}, + {"SyncListRepos", SyncListRepos, "com.atproto.sync"}, + {"SyncListReposByCollection", SyncListReposByCollection, "com.atproto.sync"}, + {"SyncSubscribeRepos", SyncSubscribeRepos, "com.atproto.sync"}, + {"SyncGetRepoStatus", SyncGetRepoStatus, "com.atproto.sync"}, + {"SyncRequestCrawl", SyncRequestCrawl, "com.atproto.sync"}, + + // ATProto server endpoints + {"ServerGetServiceAuth", ServerGetServiceAuth, "com.atproto.server"}, + {"ServerDescribeServer", ServerDescribeServer, "com.atproto.server"}, + {"ServerCreateSession", ServerCreateSession, "com.atproto.server"}, + {"ServerRefreshSession", ServerRefreshSession, "com.atproto.server"}, + {"ServerGetSession", ServerGetSession, "com.atproto.server"}, + + // ATProto repo endpoints + {"RepoDescribeRepo", RepoDescribeRepo, "com.atproto.repo"}, + {"RepoPutRecord", RepoPutRecord, "com.atproto.repo"}, + {"RepoGetRecord", RepoGetRecord, "com.atproto.repo"}, + {"RepoListRecords", RepoListRecords, "com.atproto.repo"}, + {"RepoDeleteRecord", RepoDeleteRecord, "com.atproto.repo"}, + {"RepoUploadBlob", RepoUploadBlob, "com.atproto.repo"}, + + // ATProto identity endpoints + {"IdentityResolveHandle", IdentityResolveHandle, "com.atproto.identity"}, + + // Bluesky app endpoints + {"ActorGetProfile", ActorGetProfile, "app.bsky.actor"}, + {"ActorGetProfiles", ActorGetProfiles, "app.bsky.actor"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Check that endpoint starts with /xrpc/ + if !strings.HasPrefix(tt.endpoint, "/xrpc/") { + t.Errorf("%s = %q, does not start with /xrpc/", tt.name, tt.endpoint) + } + + // Check that endpoint contains the expected namespace prefix + if !strings.Contains(tt.endpoint, tt.prefix) { + t.Errorf("%s = %q, does not contain expected prefix %q", tt.name, tt.endpoint, tt.prefix) + } + + // Check that endpoint is not empty + if tt.endpoint == "" { + t.Errorf("%s is empty", tt.name) + } + + // Check that endpoint follows naming convention: /xrpc/{namespace}.{method} + // Should have at least 3 parts after /xrpc/: namespace.namespace.method + parts := strings.Split(strings.TrimPrefix(tt.endpoint, "/xrpc/"), ".") + if len(parts) < 3 { + t.Errorf("%s = %q, does not follow XRPC convention (expected at least 3 dot-separated parts)", tt.name, tt.endpoint) + } + + // Check that method name (last part) is camelCase and not empty + method := parts[len(parts)-1] + if method == "" { + t.Errorf("%s = %q, has empty method name", tt.name, tt.endpoint) + } + if !isLowerCamelCase(method) { + t.Errorf("%s = %q, method %q is not in camelCase", tt.name, tt.endpoint, method) + } + }) + } +} + +// TestEndpointUniqueness ensures no duplicate endpoint paths +func TestEndpointUniqueness(t *testing.T) { + endpoints := []string{ + HoldInitiateUpload, + HoldGetPartUploadURL, + HoldUploadPart, + HoldCompleteUpload, + HoldAbortUpload, + HoldNotifyManifest, + HoldRequestCrew, + SyncGetBlob, + SyncGetRepo, + SyncGetRecord, + SyncListRepos, + SyncListReposByCollection, + SyncSubscribeRepos, + SyncGetRepoStatus, + SyncRequestCrawl, + ServerGetServiceAuth, + ServerDescribeServer, + ServerCreateSession, + ServerRefreshSession, + ServerGetSession, + RepoDescribeRepo, + RepoPutRecord, + RepoGetRecord, + RepoListRecords, + RepoDeleteRecord, + RepoUploadBlob, + IdentityResolveHandle, + ActorGetProfile, + ActorGetProfiles, + } + + seen := make(map[string]bool) + for _, endpoint := range endpoints { + if seen[endpoint] { + t.Errorf("Duplicate endpoint found: %q", endpoint) + } + seen[endpoint] = true + } +} + +// TestEndpointNamespaces validates that endpoints are correctly grouped by namespace +func TestEndpointNamespaces(t *testing.T) { + tests := []struct { + name string + endpoints []string + namespace string + }{ + { + name: "io.atcr.hold namespace", + endpoints: []string{ + HoldInitiateUpload, + HoldGetPartUploadURL, + HoldUploadPart, + HoldCompleteUpload, + HoldAbortUpload, + HoldNotifyManifest, + HoldRequestCrew, + }, + namespace: "io.atcr.hold", + }, + { + name: "com.atproto.sync namespace", + endpoints: []string{ + SyncGetBlob, + SyncGetRepo, + SyncGetRecord, + SyncListRepos, + SyncListReposByCollection, + SyncSubscribeRepos, + SyncGetRepoStatus, + SyncRequestCrawl, + }, + namespace: "com.atproto.sync", + }, + { + name: "com.atproto.server namespace", + endpoints: []string{ + ServerGetServiceAuth, + ServerDescribeServer, + ServerCreateSession, + ServerRefreshSession, + ServerGetSession, + }, + namespace: "com.atproto.server", + }, + { + name: "com.atproto.repo namespace", + endpoints: []string{ + RepoDescribeRepo, + RepoPutRecord, + RepoGetRecord, + RepoListRecords, + RepoDeleteRecord, + RepoUploadBlob, + }, + namespace: "com.atproto.repo", + }, + { + name: "com.atproto.identity namespace", + endpoints: []string{ + IdentityResolveHandle, + }, + namespace: "com.atproto.identity", + }, + { + name: "app.bsky.actor namespace", + endpoints: []string{ + ActorGetProfile, + ActorGetProfiles, + }, + namespace: "app.bsky.actor", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, endpoint := range tt.endpoints { + if !strings.Contains(endpoint, tt.namespace) { + t.Errorf("Endpoint %q should be in namespace %q", endpoint, tt.namespace) + } + } + }) + } +} + +// TestSpecificEndpoints validates specific endpoint paths are correct +func TestSpecificEndpoints(t *testing.T) { + tests := []struct { + name string + got string + expected string + }{ + // Spot check a few critical endpoints + {"HoldInitiateUpload", HoldInitiateUpload, "/xrpc/io.atcr.hold.initiateUpload"}, + {"SyncGetBlob", SyncGetBlob, "/xrpc/com.atproto.sync.getBlob"}, + {"ServerGetServiceAuth", ServerGetServiceAuth, "/xrpc/com.atproto.server.getServiceAuth"}, + {"RepoPutRecord", RepoPutRecord, "/xrpc/com.atproto.repo.putRecord"}, + {"IdentityResolveHandle", IdentityResolveHandle, "/xrpc/com.atproto.identity.resolveHandle"}, + {"ActorGetProfile", ActorGetProfile, "/xrpc/app.bsky.actor.getProfile"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.got != tt.expected { + t.Errorf("%s = %q, expected %q", tt.name, tt.got, tt.expected) + } + }) + } +} + +// isLowerCamelCase checks if a string follows lowerCamelCase convention +func isLowerCamelCase(s string) bool { + if len(s) == 0 { + return false + } + // First character should be lowercase + if s[0] < 'a' || s[0] > 'z' { + return false + } + // Should not contain underscores or hyphens (common in other naming conventions) + if strings.Contains(s, "_") || strings.Contains(s, "-") { + return false + } + return true +} diff --git a/pkg/atproto/lexicon_test.go b/pkg/atproto/lexicon_test.go index eb2b810..79d3b92 100644 --- a/pkg/atproto/lexicon_test.go +++ b/pkg/atproto/lexicon_test.go @@ -953,3 +953,335 @@ func TestStarSubject_JSONSerialization(t *testing.T) { t.Errorf("Repository = %v, want %v", decoded.Repository, subject.Repository) } } + +func TestRepositoryTagToRKey(t *testing.T) { + tests := []struct { + name string + repository string + tag string + want string + }{ + { + name: "simple repository and tag", + repository: "myapp", + tag: "latest", + want: "myapp_latest", + }, + { + name: "repository with slash", + repository: "org/myapp", + tag: "v1.0.0", + want: "org~myapp_v1.0.0", + }, + { + name: "multiple slashes in repository", + repository: "github.com/user/repo", + tag: "main", + want: "github.com~user~repo_main", + }, + { + name: "tag with version", + repository: "app", + tag: "v1.2.3", + want: "app_v1.2.3", + }, + { + name: "repository with hyphen", + repository: "my-app", + tag: "prod", + want: "my-app_prod", + }, + { + name: "empty repository", + repository: "", + tag: "latest", + want: "_latest", + }, + { + name: "empty tag", + repository: "myapp", + tag: "", + want: "myapp_", + }, + { + name: "both empty", + repository: "", + tag: "", + want: "_", + }, + { + name: "complex repository with slash", + repository: "namespace/app", + tag: "v2.0", + want: "namespace~app_v2.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := RepositoryTagToRKey(tt.repository, tt.tag) + if got != tt.want { + t.Errorf("RepositoryTagToRKey(%q, %q) = %q, want %q", tt.repository, tt.tag, got, tt.want) + } + }) + } +} + +func TestRKeyToRepositoryTag(t *testing.T) { + tests := []struct { + name string + rkey string + wantRepository string + wantTag string + }{ + { + name: "simple rkey", + rkey: "myapp_latest", + wantRepository: "myapp", + wantTag: "latest", + }, + { + name: "repository with tilde (encoded slash)", + rkey: "org~myapp_v1.0.0", + wantRepository: "org/myapp", + wantTag: "v1.0.0", + }, + { + name: "multiple tildes", + rkey: "github.com~user~repo_main", + wantRepository: "github.com/user/repo", + wantTag: "main", + }, + { + name: "tag with underscore (splits on last underscore)", + rkey: "app_tag_with_underscore", + wantRepository: "app_tag_with", + wantTag: "underscore", + }, + { + name: "repository with hyphen", + rkey: "my-app_prod", + wantRepository: "my-app", + wantTag: "prod", + }, + { + name: "no underscore (treats as tag)", + rkey: "justtext", + wantRepository: "", + wantTag: "justtext", + }, + { + name: "empty repository", + rkey: "_latest", + wantRepository: "", + wantTag: "latest", + }, + { + name: "empty tag", + rkey: "myapp_", + wantRepository: "myapp", + wantTag: "", + }, + { + name: "complex with tilde and multiple underscores", + rkey: "namespace~app_tag_with_underscore", + wantRepository: "namespace/app_tag_with", + wantTag: "underscore", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRepository, gotTag := RKeyToRepositoryTag(tt.rkey) + if gotRepository != tt.wantRepository { + t.Errorf("RKeyToRepositoryTag(%q) repository = %q, want %q", tt.rkey, gotRepository, tt.wantRepository) + } + if gotTag != tt.wantTag { + t.Errorf("RKeyToRepositoryTag(%q) tag = %q, want %q", tt.rkey, gotTag, tt.wantTag) + } + }) + } +} + +func TestRepositoryTagRoundTrip(t *testing.T) { + // Test that converting to rkey and back gives original values + tests := []struct { + name string + repository string + tag string + }{ + {"simple", "myapp", "latest"}, + {"with slash", "org/myapp", "v1.0.0"}, + {"multiple slashes", "github.com/user/repo", "main"}, + {"with hyphen", "my-app", "prod"}, + {"empty repository", "", "latest"}, + {"empty tag", "myapp", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Convert to rkey + rkey := RepositoryTagToRKey(tt.repository, tt.tag) + + // Convert back + gotRepository, gotTag := RKeyToRepositoryTag(rkey) + + // Verify round-trip + if gotRepository != tt.repository { + t.Errorf("Round-trip repository = %q, want %q (via rkey %q)", gotRepository, tt.repository, rkey) + } + if gotTag != tt.tag { + t.Errorf("Round-trip tag = %q, want %q (via rkey %q)", gotTag, tt.tag, rkey) + } + }) + } +} + +func TestNewLayerRecord(t *testing.T) { + tests := []struct { + name string + digest string + size int64 + mediaType string + repository string + userDID string + userHandle string + }{ + { + name: "standard layer", + digest: "sha256:abc123", + size: 1024, + mediaType: "application/vnd.oci.image.layer.v1.tar+gzip", + repository: "myapp", + userDID: "did:plc:user123", + userHandle: "alice.bsky.social", + }, + { + name: "large layer", + digest: "sha256:def456", + size: 1073741824, // 1GB + mediaType: "application/vnd.oci.image.layer.v1.tar+gzip", + repository: "largeapp", + userDID: "did:plc:user456", + userHandle: "bob.example.com", + }, + { + name: "empty values", + digest: "", + size: 0, + mediaType: "", + repository: "", + userDID: "", + userHandle: "", + }, + { + name: "config layer", + digest: "sha256:config123", + size: 512, + mediaType: "application/vnd.oci.image.config.v1+json", + repository: "app/subapp", + userDID: "did:web:example.com", + userHandle: "charlie.tangled.io", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + record := NewLayerRecord(tt.digest, tt.size, tt.mediaType, tt.repository, tt.userDID, tt.userHandle) + + // Verify all fields + if record == nil { + t.Fatal("NewLayerRecord() returned nil") + } + + if record.Type != LayerCollection { + t.Errorf("Type = %q, want %q", record.Type, LayerCollection) + } + + if record.Digest != tt.digest { + t.Errorf("Digest = %q, want %q", record.Digest, tt.digest) + } + + if record.Size != tt.size { + t.Errorf("Size = %d, want %d", record.Size, tt.size) + } + + if record.MediaType != tt.mediaType { + t.Errorf("MediaType = %q, want %q", record.MediaType, tt.mediaType) + } + + if record.Repository != tt.repository { + t.Errorf("Repository = %q, want %q", record.Repository, tt.repository) + } + + if record.UserDID != tt.userDID { + t.Errorf("UserDID = %q, want %q", record.UserDID, tt.userDID) + } + + if record.UserHandle != tt.userHandle { + t.Errorf("UserHandle = %q, want %q", record.UserHandle, tt.userHandle) + } + + // Verify CreatedAt is set and is a valid RFC3339 timestamp + if record.CreatedAt == "" { + t.Error("CreatedAt is empty") + } + + // Parse to verify it's a valid timestamp + _, err := time.Parse(time.RFC3339, record.CreatedAt) + if err != nil { + t.Errorf("CreatedAt %q is not a valid RFC3339 timestamp: %v", record.CreatedAt, err) + } + }) + } +} + +func TestNewLayerRecordJSON(t *testing.T) { + // Test that LayerRecord can be marshaled/unmarshaled to/from JSON + record := NewLayerRecord( + "sha256:abc123", + 1024, + "application/vnd.oci.image.layer.v1.tar+gzip", + "myapp", + "did:plc:user123", + "alice.bsky.social", + ) + + // Marshal to JSON + jsonData, err := json.Marshal(record) + if err != nil { + t.Fatalf("json.Marshal() error = %v", err) + } + + // Unmarshal back + var decoded LayerRecord + if err := json.Unmarshal(jsonData, &decoded); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + + // Verify fields match + if decoded.Type != record.Type { + t.Errorf("Type = %q, want %q", decoded.Type, record.Type) + } + if decoded.Digest != record.Digest { + t.Errorf("Digest = %q, want %q", decoded.Digest, record.Digest) + } + if decoded.Size != record.Size { + t.Errorf("Size = %d, want %d", decoded.Size, record.Size) + } + if decoded.MediaType != record.MediaType { + t.Errorf("MediaType = %q, want %q", decoded.MediaType, record.MediaType) + } + if decoded.Repository != record.Repository { + t.Errorf("Repository = %q, want %q", decoded.Repository, record.Repository) + } + if decoded.UserDID != record.UserDID { + t.Errorf("UserDID = %q, want %q", decoded.UserDID, record.UserDID) + } + if decoded.UserHandle != record.UserHandle { + t.Errorf("UserHandle = %q, want %q", decoded.UserHandle, record.UserHandle) + } + if decoded.CreatedAt != record.CreatedAt { + t.Errorf("CreatedAt = %q, want %q", decoded.CreatedAt, record.CreatedAt) + } +} diff --git a/pkg/atproto/utils_test.go b/pkg/atproto/utils_test.go new file mode 100644 index 0000000..849082c --- /dev/null +++ b/pkg/atproto/utils_test.go @@ -0,0 +1,189 @@ +package atproto + +import "testing" + +func TestResolveHoldURL(t *testing.T) { + tests := []struct { + name string + holdIdentifier string + want string + }{ + // URL passthrough tests + { + name: "http URL passthrough", + holdIdentifier: "http://hold.example.com", + want: "http://hold.example.com", + }, + { + name: "https URL passthrough", + holdIdentifier: "https://hold.example.com", + want: "https://hold.example.com", + }, + { + name: "http URL with port passthrough", + holdIdentifier: "http://hold.example.com:8080", + want: "http://hold.example.com:8080", + }, + { + name: "https URL with port passthrough", + holdIdentifier: "https://hold.example.com:8443", + want: "https://hold.example.com:8443", + }, + { + name: "http URL with path passthrough", + holdIdentifier: "http://hold.example.com/some/path", + want: "http://hold.example.com/some/path", + }, + + // did:web to HTTPS (domain names) + { + name: "did:web domain to https", + holdIdentifier: "did:web:hold01.atcr.io", + want: "https://hold01.atcr.io", + }, + { + name: "did:web subdomain to https", + holdIdentifier: "did:web:my-hold.example.com", + want: "https://my-hold.example.com", + }, + { + name: "did:web simple domain to https", + holdIdentifier: "did:web:example.com", + want: "https://example.com", + }, + + // did:web to HTTP (ports) + { + name: "did:web with port to http", + holdIdentifier: "did:web:172.28.0.3:8080", + want: "http://172.28.0.3:8080", + }, + { + name: "did:web domain with port to http", + holdIdentifier: "did:web:hold.example.com:8080", + want: "http://hold.example.com:8080", + }, + { + name: "did:web localhost with port to http", + holdIdentifier: "did:web:localhost:8080", + want: "http://localhost:8080", + }, + + // did:web to HTTP (localhost) + { + name: "did:web localhost to http", + holdIdentifier: "did:web:localhost", + want: "http://localhost", + }, + + // did:web to HTTP (127.0.0.1) + { + name: "did:web 127.0.0.1 to http", + holdIdentifier: "did:web:127.0.0.1", + want: "http://127.0.0.1", + }, + { + name: "did:web 127.0.0.1 with port to http", + holdIdentifier: "did:web:127.0.0.1:8080", + want: "http://127.0.0.1:8080", + }, + + // did:web to HTTP (IP addresses) + { + name: "did:web IPv4 address to http", + holdIdentifier: "did:web:192.168.1.1", + want: "http://192.168.1.1", + }, + { + name: "did:web IPv4 with port to http", + holdIdentifier: "did:web:10.0.0.5:3000", + want: "http://10.0.0.5:3000", + }, + { + name: "did:web private IP to http", + holdIdentifier: "did:web:172.16.0.1", + want: "http://172.16.0.1", + }, + + // Fallback behavior (plain hostname) + { + name: "plain hostname fallback to https", + holdIdentifier: "hold.example.com", + want: "https://hold.example.com", + }, + { + name: "plain single word fallback to https", + holdIdentifier: "myhold", + want: "https://myhold", + }, + + // Edge cases + { + name: "empty string fallback", + holdIdentifier: "", + want: "https://", + }, + { + name: "did:web empty hostname", + holdIdentifier: "did:web:", + want: "https://", + }, + { + name: "just did:web prefix", + holdIdentifier: "did:web", + want: "https://did:web", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ResolveHoldURL(tt.holdIdentifier) + if got != tt.want { + t.Errorf("ResolveHoldURL(%q) = %q, want %q", tt.holdIdentifier, got, tt.want) + } + }) + } +} + +// TestResolveHoldURLRoundTrip tests that converting back and forth works +func TestResolveHoldURLRoundTrip(t *testing.T) { + tests := []struct { + name string + input string + wantHTTP bool // true if result should be http, false for https + }{ + {"domain to https and idempotent", "did:web:hold.atcr.io", false}, + {"IP to http and idempotent", "did:web:192.168.1.1", true}, + {"port to http and idempotent", "did:web:example.com:8080", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // First conversion + first := ResolveHoldURL(tt.input) + + // Second conversion (should be idempotent since output is URL) + second := ResolveHoldURL(first) + + if first != second { + t.Errorf("ResolveHoldURL is not idempotent: first=%q, second=%q", first, second) + } + + // Verify correct protocol + if tt.wantHTTP { + if !hasPrefix(first, "http://") { + t.Errorf("Expected http:// prefix, got %q", first) + } + } else { + if !hasPrefix(first, "https://") { + t.Errorf("Expected https:// prefix, got %q", first) + } + } + }) + } +} + +// Helper function to check prefix +func hasPrefix(s, prefix string) bool { + return len(s) >= len(prefix) && s[:len(prefix)] == prefix +} diff --git a/pkg/auth/hold_remote_test.go b/pkg/auth/hold_remote_test.go index 07f23be..bbd42c3 100644 --- a/pkg/auth/hold_remote_test.go +++ b/pkg/auth/hold_remote_test.go @@ -45,7 +45,7 @@ func TestNewRemoteHoldAuthorizer_TestMode(t *testing.T) { // setupTestDB creates an in-memory database for testing func setupTestDB(t *testing.T) *sql.DB { - testDB, err := db.InitDB(":memory:") + testDB, err := db.InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } diff --git a/pkg/auth/oauth/browser.go b/pkg/auth/oauth/browser.go index bc57425..15daddc 100644 --- a/pkg/auth/oauth/browser.go +++ b/pkg/auth/oauth/browser.go @@ -6,20 +6,46 @@ import ( "runtime" ) -// OpenBrowser opens the default browser to the given URL -func OpenBrowser(url string) error { - var cmd *exec.Cmd +// CommandExecutor is an interface for executing system commands. +// This allows for dependency injection and mocking in tests. +type CommandExecutor interface { + Execute(name string, args ...string) error +} + +// realCommandExecutor is the production implementation that actually executes commands. +type realCommandExecutor struct{} + +func (e *realCommandExecutor) Execute(name string, args ...string) error { + return exec.Command(name, args...).Start() +} - switch runtime.GOOS { +// buildBrowserCommand returns the command and arguments needed to open a browser on the given OS. +// This is a pure function with no side effects, making it easily testable. +func buildBrowserCommand(goos, url string) (string, []string, error) { + switch goos { case "darwin": - cmd = exec.Command("open", url) + return "open", []string{url}, nil case "linux": - cmd = exec.Command("xdg-open", url) + return "xdg-open", []string{url}, nil case "windows": - cmd = exec.Command("rundll32", "url.dll,FileProtocolHandler", url) + return "rundll32", []string{"url.dll,FileProtocolHandler", url}, nil default: - return fmt.Errorf("unsupported platform: %s", runtime.GOOS) + return "", nil, fmt.Errorf("unsupported platform: %s", goos) } +} - return cmd.Start() +// openBrowserWithExecutor opens the browser using the provided executor. +// This allows for dependency injection in tests. +func openBrowserWithExecutor(goos, url string, executor CommandExecutor) error { + cmd, args, err := buildBrowserCommand(goos, url) + if err != nil { + return err + } + return executor.Execute(cmd, args...) +} + +// OpenBrowser opens the default browser to the given URL. +// This is the public API that maintains backward compatibility. +func OpenBrowser(url string) error { + return openBrowserWithExecutor(runtime.GOOS, url, &realCommandExecutor{}) } diff --git a/pkg/auth/oauth/browser_test.go b/pkg/auth/oauth/browser_test.go index 98017d9..bb9f32a 100644 --- a/pkg/auth/oauth/browser_test.go +++ b/pkg/auth/oauth/browser_test.go @@ -1,29 +1,242 @@ package oauth import ( - "runtime" + "fmt" + "strings" "testing" ) -func TestOpenBrowser_OSSupport(t *testing.T) { - // Test that we handle different operating systems - // We don't actually call OpenBrowser to avoid opening real browsers during tests - - validOSes := map[string]bool{ - "darwin": true, - "linux": true, - "windows": true, - } - - if !validOSes[runtime.GOOS] { - t.Skipf("Unsupported OS for browser testing: %s", runtime.GOOS) - } - - // Just verify the function exists and doesn't panic with basic validation - // We skip actually calling it to avoid opening user's browser during tests - t.Logf("OpenBrowser is available for OS: %s", runtime.GOOS) +// mockCommandExecutor is a test mock that records executed commands without actually running them. +type mockCommandExecutor struct { + executedCmd string + executedArgs []string + returnError error } -// Note: Full browser opening tests would require mocking exec.Command -// or running in a headless environment. Skipping actual browser launch -// to avoid disrupting test runs. +func (m *mockCommandExecutor) Execute(name string, args ...string) error { + m.executedCmd = name + m.executedArgs = args + return m.returnError +} + +func TestBuildBrowserCommand(t *testing.T) { + tests := []struct { + name string + goos string + url string + wantCmd string + wantArgs []string + wantErr bool + errContains string + }{ + { + name: "macOS with simple URL", + goos: "darwin", + url: "https://example.com", + wantCmd: "open", + wantArgs: []string{"https://example.com"}, + wantErr: false, + }, + { + name: "Linux with simple URL", + goos: "linux", + url: "https://example.com", + wantCmd: "xdg-open", + wantArgs: []string{"https://example.com"}, + wantErr: false, + }, + { + name: "Windows with simple URL", + goos: "windows", + url: "https://example.com", + wantCmd: "rundll32", + wantArgs: []string{"url.dll,FileProtocolHandler", "https://example.com"}, + wantErr: false, + }, + { + name: "macOS with URL containing query params", + goos: "darwin", + url: "https://example.com/callback?code=123&state=abc", + wantCmd: "open", + wantArgs: []string{"https://example.com/callback?code=123&state=abc"}, + wantErr: false, + }, + { + name: "Linux with URL containing fragment", + goos: "linux", + url: "https://example.com/page#section", + wantCmd: "xdg-open", + wantArgs: []string{"https://example.com/page#section"}, + wantErr: false, + }, + { + name: "Windows with URL containing special chars", + goos: "windows", + url: "https://example.com/path?key=value&other=123", + wantCmd: "rundll32", + wantArgs: []string{"url.dll,FileProtocolHandler", "https://example.com/path?key=value&other=123"}, + wantErr: false, + }, + { + name: "unsupported OS", + goos: "freebsd", + url: "https://example.com", + wantCmd: "", + wantArgs: nil, + wantErr: true, + errContains: "unsupported platform", + }, + { + name: "unknown OS", + goos: "amiga", + url: "https://example.com", + wantCmd: "", + wantArgs: nil, + wantErr: true, + errContains: "amiga", + }, + { + name: "empty URL on macOS", + goos: "darwin", + url: "", + wantCmd: "open", + wantArgs: []string{""}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd, args, err := buildBrowserCommand(tt.goos, tt.url) + + // Check error + if tt.wantErr { + if err == nil { + t.Errorf("buildBrowserCommand() expected error, got nil") + return + } + if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("buildBrowserCommand() error = %v, should contain %q", err, tt.errContains) + } + return + } + + if err != nil { + t.Errorf("buildBrowserCommand() unexpected error = %v", err) + return + } + + // Check command + if cmd != tt.wantCmd { + t.Errorf("buildBrowserCommand() cmd = %v, want %v", cmd, tt.wantCmd) + } + + // Check args + if len(args) != len(tt.wantArgs) { + t.Errorf("buildBrowserCommand() args length = %d, want %d", len(args), len(tt.wantArgs)) + return + } + for i, arg := range args { + if arg != tt.wantArgs[i] { + t.Errorf("buildBrowserCommand() args[%d] = %v, want %v", i, arg, tt.wantArgs[i]) + } + } + }) + } +} + +func TestOpenBrowserWithExecutor(t *testing.T) { + tests := []struct { + name string + goos string + url string + executorError error + wantCmd string + wantArgs []string + wantErr bool + errContains string + }{ + { + name: "macOS success", + goos: "darwin", + url: "https://example.com", + wantCmd: "open", + wantArgs: []string{"https://example.com"}, + wantErr: false, + }, + { + name: "Linux success", + goos: "linux", + url: "https://example.com/auth", + wantCmd: "xdg-open", + wantArgs: []string{"https://example.com/auth"}, + wantErr: false, + }, + { + name: "Windows success", + goos: "windows", + url: "https://example.com/callback?code=123", + wantCmd: "rundll32", + wantArgs: []string{"url.dll,FileProtocolHandler", "https://example.com/callback?code=123"}, + wantErr: false, + }, + { + name: "unsupported OS", + goos: "plan9", + url: "https://example.com", + wantErr: true, + errContains: "unsupported platform", + }, + { + name: "executor error", + goos: "darwin", + url: "https://example.com", + executorError: fmt.Errorf("exec failed"), + wantCmd: "open", + wantArgs: []string{"https://example.com"}, + wantErr: true, + errContains: "exec failed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockCommandExecutor{returnError: tt.executorError} + + err := openBrowserWithExecutor(tt.goos, tt.url, mock) + + // Check error + if tt.wantErr { + if err == nil { + t.Errorf("openBrowserWithExecutor() expected error, got nil") + return + } + if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("openBrowserWithExecutor() error = %v, should contain %q", err, tt.errContains) + } + return + } + + if err != nil { + t.Errorf("openBrowserWithExecutor() unexpected error = %v", err) + return + } + + // Verify mock was called with correct command + if mock.executedCmd != tt.wantCmd { + t.Errorf("executed command = %v, want %v", mock.executedCmd, tt.wantCmd) + } + + // Verify mock was called with correct args + if len(mock.executedArgs) != len(tt.wantArgs) { + t.Errorf("executed args length = %d, want %d", len(mock.executedArgs), len(tt.wantArgs)) + return + } + for i, arg := range mock.executedArgs { + if arg != tt.wantArgs[i] { + t.Errorf("executed args[%d] = %v, want %v", i, arg, tt.wantArgs[i]) + } + } + }) + } +} diff --git a/pkg/auth/token/handler_test.go b/pkg/auth/token/handler_test.go index 90a0375..1ba9859 100644 --- a/pkg/auth/token/handler_test.go +++ b/pkg/auth/token/handler_test.go @@ -2,23 +2,59 @@ package token import ( "context" + "crypto/rsa" "crypto/tls" "database/sql" "encoding/base64" "encoding/json" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" + "sync" "testing" "time" "atcr.io/pkg/appview/db" ) +// Shared test key to avoid generating a new RSA key for each test +// Generating a 2048-bit RSA key takes ~0.15s, so reusing one key saves ~4.5s for 32 tests +var ( + sharedTestKey *rsa.PrivateKey + sharedTestKeyPath string + sharedTestKeyOnce sync.Once + sharedTestKeyDir string +) + +// getSharedTestKey returns a shared RSA key and its file path for all tests +// The key is generated once and reused across all tests in this package +func getSharedTestKey(t *testing.T) string { + sharedTestKeyOnce.Do(func() { + // Create a persistent temp directory for the shared key + var err error + sharedTestKeyDir, err = os.MkdirTemp("", "atcr-test-keys-*") + if err != nil { + t.Fatalf("Failed to create test key directory: %v", err) + } + + sharedTestKeyPath = filepath.Join(sharedTestKeyDir, "test-key.pem") + + // Generate the key once (this is the expensive operation we want to avoid repeating) + // This will also generate the certificate via NewIssuer + _, err = NewIssuer(sharedTestKeyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("Failed to generate shared test key: %v", err) + } + }) + + return sharedTestKeyPath +} + // setupTestDeviceStore creates an in-memory SQLite database for testing func setupTestDeviceStore(t *testing.T) (*db.DeviceStore, *sql.DB) { - testDB, err := db.InitDB(":memory:") + testDB, err := db.InitDB(":memory:", true) if err != nil { t.Fatalf("Failed to initialize test database: %v", err) } @@ -55,8 +91,7 @@ func createTestDevice(t *testing.T, store *db.DeviceStore, testDB *sql.DB, did, } func TestNewHandler(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -78,8 +113,7 @@ func TestNewHandler(t *testing.T) { } func TestHandler_SetPostAuthCallback(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -98,8 +132,7 @@ func TestHandler_SetPostAuthCallback(t *testing.T) { } func TestHandler_ServeHTTP_NoAuth(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -124,8 +157,7 @@ func TestHandler_ServeHTTP_NoAuth(t *testing.T) { } func TestHandler_ServeHTTP_WrongMethod(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -146,8 +178,7 @@ func TestHandler_ServeHTTP_WrongMethod(t *testing.T) { } func TestHandler_ServeHTTP_DeviceAuth_Valid(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -197,8 +228,7 @@ func TestHandler_ServeHTTP_DeviceAuth_Valid(t *testing.T) { } func TestHandler_ServeHTTP_DeviceAuth_Invalid(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -222,8 +252,7 @@ func TestHandler_ServeHTTP_DeviceAuth_Invalid(t *testing.T) { } func TestHandler_ServeHTTP_InvalidScope(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -253,8 +282,7 @@ func TestHandler_ServeHTTP_InvalidScope(t *testing.T) { } func TestHandler_ServeHTTP_AccessDenied(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -284,8 +312,7 @@ func TestHandler_ServeHTTP_AccessDenied(t *testing.T) { } func TestHandler_ServeHTTP_WithCallback(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -319,8 +346,7 @@ func TestHandler_ServeHTTP_WithCallback(t *testing.T) { } func TestHandler_ServeHTTP_MultipleScopes(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -346,8 +372,7 @@ func TestHandler_ServeHTTP_MultipleScopes(t *testing.T) { } func TestHandler_ServeHTTP_WildcardScope(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -372,8 +397,7 @@ func TestHandler_ServeHTTP_WildcardScope(t *testing.T) { } func TestHandler_ServeHTTP_NoScope(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -512,8 +536,7 @@ func TestTokenResponse_JSONFormat(t *testing.T) { } func TestHandler_ServeHTTP_AuthHeader(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -537,8 +560,7 @@ func TestHandler_ServeHTTP_AuthHeader(t *testing.T) { } func TestHandler_ServeHTTP_ContentType(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -567,8 +589,7 @@ func TestHandler_ServeHTTP_ContentType(t *testing.T) { } func TestHandler_ServeHTTP_ExpiresIn(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) // Create issuer with specific expiration expiration := 10 * time.Minute @@ -600,8 +621,7 @@ func TestHandler_ServeHTTP_ExpiresIn(t *testing.T) { } func TestHandler_ServeHTTP_PullOnlyAccess(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { diff --git a/pkg/auth/token/issuer_test.go b/pkg/auth/token/issuer_test.go index 8d7ab49..dc4ed7a 100644 --- a/pkg/auth/token/issuer_test.go +++ b/pkg/auth/token/issuer_test.go @@ -16,6 +16,38 @@ import ( "github.com/golang-jwt/jwt/v5" ) +// Shared test key to avoid generating a new RSA key for each test +// Generating a 2048-bit RSA key takes ~0.15s, so reusing one key saves significant time +var ( + issuerSharedTestKey *rsa.PrivateKey + issuerSharedTestKeyPath string + issuerSharedTestKeyOnce sync.Once + issuerSharedTestKeyDir string +) + +// getSharedTestKey returns a shared RSA key and its file path for all tests +// The key is generated once and reused across all tests in this package +func getIssuerSharedTestKey(t *testing.T) string { + issuerSharedTestKeyOnce.Do(func() { + // Create a persistent temp directory for the shared key + var err error + issuerSharedTestKeyDir, err = os.MkdirTemp("", "atcr-issuer-test-keys-*") + if err != nil { + t.Fatalf("Failed to create test key directory: %v", err) + } + + issuerSharedTestKeyPath = filepath.Join(issuerSharedTestKeyDir, "test-key.pem") + + // Generate the key once (this is the expensive operation we want to avoid repeating) + _, err = NewIssuer(issuerSharedTestKeyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("Failed to generate shared test key: %v", err) + } + }) + + return issuerSharedTestKeyPath +} + func TestNewIssuer_GeneratesKey(t *testing.T) { tmpDir := t.TempDir() keyPath := filepath.Join(tmpDir, "private-key.pem") @@ -102,8 +134,7 @@ func TestNewIssuer_LoadsExistingKey(t *testing.T) { } func TestIssuer_Issue(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -136,8 +167,7 @@ func TestIssuer_Issue(t *testing.T) { } func TestIssuer_Issue_EmptyAccess(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -155,8 +185,7 @@ func TestIssuer_Issue_EmptyAccess(t *testing.T) { } func TestIssuer_Issue_ValidateToken(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -235,8 +264,7 @@ func TestIssuer_Issue_ValidateToken(t *testing.T) { } func TestIssuer_Issue_X5CHeader(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -304,8 +332,7 @@ func TestIssuer_Issue_X5CHeader(t *testing.T) { } func TestIssuer_PublicKey(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -328,8 +355,7 @@ func TestIssuer_PublicKey(t *testing.T) { } func TestIssuer_Expiration(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) expiration := 30 * time.Minute issuer, err := NewIssuer(keyPath, "atcr.io", "registry", expiration) @@ -343,8 +369,7 @@ func TestIssuer_Expiration(t *testing.T) { } func TestIssuer_ConcurrentIssue(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) if err != nil { @@ -537,8 +562,7 @@ func TestIssuer_DifferentExpirations(t *testing.T) { for _, expiration := range expirations { t.Run(expiration.String(), func(t *testing.T) { - tmpDir := t.TempDir() - keyPath := filepath.Join(tmpDir, "private-key.pem") + keyPath := getIssuerSharedTestKey(t) issuer, err := NewIssuer(keyPath, "atcr.io", "registry", expiration) if err != nil {