From 79d1126726156ad400d720e01df3b5f1e8e06534 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sat, 20 Dec 2025 21:50:09 -0600 Subject: [PATCH] better handling for io.atcr.repo.page --- lexicons/io/atcr/repo/page.json | 2 +- pkg/appview/middleware/registry.go | 31 +--- pkg/appview/middleware/registry_test.go | 54 ------ pkg/appview/storage/context.go | 8 +- pkg/appview/storage/manifest_store.go | 230 ++++++++++++++++++++++-- pkg/atproto/lexicon.go | 13 -- pkg/atproto/lexicon_test.go | 50 ------ pkg/auth/oauth/client.go | 1 + 8 files changed, 231 insertions(+), 158 deletions(-) diff --git a/lexicons/io/atcr/repo/page.json b/lexicons/io/atcr/repo/page.json index 95db9b9..5e4a02a 100644 --- a/lexicons/io/atcr/repo/page.json +++ b/lexicons/io/atcr/repo/page.json @@ -24,7 +24,7 @@ "type": "blob", "description": "Repository avatar/icon image.", "accept": ["image/png", "image/jpeg", "image/webp"], - "maxSize": 1000000 + "maxSize": 3000000 }, "createdAt": { "type": "string", diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index ac8dc0a..c12957b 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -2,7 +2,6 @@ package middleware import ( "context" - "encoding/json" "fmt" "log/slog" "net/http" @@ -16,6 +15,7 @@ import ( "github.com/distribution/distribution/v3/registry/storage/driver" "github.com/distribution/reference" + "atcr.io/pkg/appview/readme" "atcr.io/pkg/appview/storage" "atcr.io/pkg/atproto" "atcr.io/pkg/auth" @@ -208,6 +208,7 @@ type NamespaceResolver struct { database storage.DatabaseMetrics // Metrics database (copied from global on init) authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init) validationCache *validationCache // Request-level service token cache + readmeFetcher *readme.Fetcher // README fetcher for repo pages } // initATProtoResolver initializes the name resolution middleware @@ -242,6 +243,7 @@ func initATProtoResolver(ctx context.Context, ns distribution.Namespace, _ drive database: globalDatabase, authorizer: globalAuthorizer, validationCache: newValidationCache(), + readmeFetcher: readme.NewFetcher(), }, nil } @@ -458,6 +460,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name Database: nr.database, Authorizer: nr.authorizer, Refresher: nr.refresher, + ReadmeFetcher: nr.readmeFetcher, } return storage.NewRoutingRepository(repo, registryCtx), nil @@ -481,8 +484,7 @@ func (nr *NamespaceResolver) BlobStatter() distribution.BlobStatter { // findHoldDID determines which hold DID to use for blob storage // Priority order: // 1. User's sailor profile defaultHold (if set) -// 2. User's own hold record (io.atcr.hold) -// 3. AppView's default hold DID +// 2. AppView's default hold DID // Returns a hold DID (e.g., "did:web:hold01.atcr.io"), or empty string if none configured func (nr *NamespaceResolver) findHoldDID(ctx context.Context, did, pdsEndpoint string) string { // Create ATProto client (without auth - reading public records) @@ -508,28 +510,7 @@ func (nr *NamespaceResolver) findHoldDID(ctx context.Context, did, pdsEndpoint s return profile.DefaultHold } - // Profile doesn't exist or defaultHold is null/empty - // Check for user's own hold records - records, err := client.ListRecords(ctx, atproto.HoldCollection, 10) - if err != nil { - // Failed to query holds, use default - return nr.defaultHoldDID - } - - // Find the first hold record - for _, record := range records { - var holdRecord atproto.HoldRecord - if err := json.Unmarshal(record.Value, &holdRecord); err != nil { - continue - } - - // Return the endpoint from the first hold (normalize to DID if URL) - if holdRecord.Endpoint != "" { - return atproto.ResolveHoldDIDFromURL(holdRecord.Endpoint) - } - } - - // No profile defaultHold and no own hold records - use AppView default + // No profile defaultHold - use AppView default return nr.defaultHoldDID } diff --git a/pkg/appview/middleware/registry_test.go b/pkg/appview/middleware/registry_test.go index 10595da..2c3d0a0 100644 --- a/pkg/appview/middleware/registry_test.go +++ b/pkg/appview/middleware/registry_test.go @@ -199,45 +199,6 @@ func TestFindHoldDID_SailorProfile(t *testing.T) { assert.Equal(t, "did:web:user.hold.io", holdDID, "should use sailor profile's defaultHold") } -// TestFindHoldDID_LegacyHoldRecords tests legacy hold record discovery -func TestFindHoldDID_LegacyHoldRecords(t *testing.T) { - // Start a mock PDS server that returns hold records - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/xrpc/com.atproto.repo.getRecord" { - // Profile not found - w.WriteHeader(http.StatusNotFound) - return - } - if r.URL.Path == "/xrpc/com.atproto.repo.listRecords" { - // Return hold record - holdRecord := atproto.NewHoldRecord("https://legacy.hold.io", "alice", true) - recordJSON, _ := json.Marshal(holdRecord) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "records": []any{ - map[string]any{ - "uri": "at://did:plc:test123/io.atcr.hold/abc123", - "value": json.RawMessage(recordJSON), - }, - }, - }) - return - } - w.WriteHeader(http.StatusNotFound) - })) - defer mockPDS.Close() - - resolver := &NamespaceResolver{ - defaultHoldDID: "did:web:default.atcr.io", - } - - ctx := context.Background() - holdDID := resolver.findHoldDID(ctx, "did:plc:test123", mockPDS.URL) - - // Legacy URL should be converted to DID - assert.Equal(t, "did:web:legacy.hold.io", holdDID, "should use legacy hold record and convert to DID") -} - // TestFindHoldDID_Priority tests the priority order func TestFindHoldDID_Priority(t *testing.T) { // Start a mock PDS server that returns both profile and hold records @@ -251,21 +212,6 @@ func TestFindHoldDID_Priority(t *testing.T) { }) return } - if r.URL.Path == "/xrpc/com.atproto.repo.listRecords" { - // Return hold record (should be ignored since profile exists) - holdRecord := atproto.NewHoldRecord("https://legacy.hold.io", "alice", true) - recordJSON, _ := json.Marshal(holdRecord) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "records": []any{ - map[string]any{ - "uri": "at://did:plc:test123/io.atcr.hold/abc123", - "value": json.RawMessage(recordJSON), - }, - }, - }) - return - } w.WriteHeader(http.StatusNotFound) })) defer mockPDS.Close() diff --git a/pkg/appview/storage/context.go b/pkg/appview/storage/context.go index 7d3cfbf..5921615 100644 --- a/pkg/appview/storage/context.go +++ b/pkg/appview/storage/context.go @@ -1,6 +1,7 @@ package storage import ( + "atcr.io/pkg/appview/readme" "atcr.io/pkg/atproto" "atcr.io/pkg/auth" "atcr.io/pkg/auth/oauth" @@ -27,7 +28,8 @@ type RegistryContext struct { AuthMethod string // Auth method used ("oauth" or "app_password") // Shared services (same for all requests) - Database DatabaseMetrics // Metrics tracking database - Authorizer auth.HoldAuthorizer // Hold access authorization - Refresher *oauth.Refresher // OAuth session manager + Database DatabaseMetrics // Metrics tracking database + Authorizer auth.HoldAuthorizer // Hold access authorization + Refresher *oauth.Refresher // OAuth session manager + ReadmeFetcher *readme.Fetcher // README fetcher for repo pages } diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 0c75a18..315525e 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -12,7 +12,9 @@ import ( "net/http" "strings" "sync" + "time" + "atcr.io/pkg/appview/readme" "atcr.io/pkg/atproto" "github.com/distribution/distribution/v3" "github.com/opencontainers/go-digest" @@ -432,13 +434,6 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr return } - // Check for relevant annotations that we can use for repo page - description := manifestRecord.Annotations["org.opencontainers.image.description"] - if description == "" { - // No description annotation - nothing to create - return - } - // Check if repo page already exists (don't overwrite user's custom content) rkey := s.ctx.Repository _, err := s.ctx.ATProtoClient.GetRecord(ctx, atproto.RepoPageCollection, rkey) @@ -454,12 +449,31 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr return } - // Create new repo page record from manifest annotations - // Note: Avatar is not extracted from annotations here - that's handled separately - // (would require uploading a blob if annotation contains a URL) - repoPage := atproto.NewRepoPageRecord(s.ctx.Repository, description, nil) + // Try to fetch README content from external sources + // Priority: io.atcr.readme annotation > derived from org.opencontainers.image.source > org.opencontainers.image.description + description := s.fetchReadmeContent(ctx, manifestRecord.Annotations) - slog.Info("Creating repo page from manifest annotations", "did", s.ctx.DID, "repository", s.ctx.Repository) + // If no README content could be fetched, fall back to description annotation + if description == "" { + description = manifestRecord.Annotations["org.opencontainers.image.description"] + } + + // Try to fetch and upload icon from io.atcr.icon annotation + var avatarRef *atproto.ATProtoBlobRef + if iconURL := manifestRecord.Annotations["io.atcr.icon"]; iconURL != "" { + avatarRef = s.fetchAndUploadIcon(ctx, iconURL) + } + + // If no description and no icon, nothing to create + if description == "" && avatarRef == nil { + slog.Debug("No README, description, or icon found for repo page", "did", s.ctx.DID, "repository", s.ctx.Repository) + return + } + + // Create new repo page record with description and optional avatar + repoPage := atproto.NewRepoPageRecord(s.ctx.Repository, description, avatarRef) + + slog.Info("Creating repo page from manifest annotations", "did", s.ctx.DID, "repository", s.ctx.Repository, "descriptionLength", len(description), "hasAvatar", avatarRef != nil) _, err = s.ctx.ATProtoClient.PutRecord(ctx, atproto.RepoPageCollection, rkey, repoPage) if err != nil { @@ -469,3 +483,195 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr slog.Info("Repo page created successfully", "did", s.ctx.DID, "repository", s.ctx.Repository) } + +// fetchReadmeContent attempts to fetch README content from external sources +// Priority: io.atcr.readme annotation > derived from org.opencontainers.image.source +// Returns the raw markdown content, or empty string if not available +func (s *ManifestStore) fetchReadmeContent(ctx context.Context, annotations map[string]string) string { + if s.ctx.ReadmeFetcher == nil { + return "" + } + + // Create a context with timeout for README fetching (don't block push too long) + fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + // Priority 1: Direct README URL from io.atcr.readme annotation + if readmeURL := annotations["io.atcr.readme"]; readmeURL != "" { + content, err := s.fetchRawReadme(fetchCtx, readmeURL) + if err != nil { + slog.Debug("Failed to fetch README from io.atcr.readme annotation", "url", readmeURL, "error", err) + } else if content != "" { + slog.Info("Fetched README from io.atcr.readme annotation", "url", readmeURL, "length", len(content)) + return content + } + } + + // Priority 2: Derive README URL from org.opencontainers.image.source + if sourceURL := annotations["org.opencontainers.image.source"]; sourceURL != "" { + // Try main branch first, then master + for _, branch := range []string{"main", "master"} { + readmeURL := readme.DeriveReadmeURL(sourceURL, branch) + if readmeURL == "" { + continue + } + + content, err := s.fetchRawReadme(fetchCtx, readmeURL) + if err != nil { + // Only log non-404 errors (404 is expected when trying main vs master) + if !readme.Is404(err) { + slog.Debug("Failed to fetch README from source URL", "url", readmeURL, "branch", branch, "error", err) + } + continue + } + + if content != "" { + slog.Info("Fetched README from source URL", "sourceURL", sourceURL, "branch", branch, "length", len(content)) + return content + } + } + } + + return "" +} + +// fetchRawReadme fetches raw markdown content from a URL +// Returns the raw markdown (not rendered HTML) for storage in the repo page record +func (s *ManifestStore) fetchRawReadme(ctx context.Context, readmeURL string) (string, error) { + // Use a simple HTTP client to fetch raw content + // We want raw markdown, not rendered HTML (the Fetcher renders to HTML) + req, err := http.NewRequestWithContext(ctx, "GET", readmeURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("User-Agent", "ATCR-README-Fetcher/1.0") + + client := &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 5 { + return fmt.Errorf("too many redirects") + } + return nil + }, + } + + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("failed to fetch URL: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("unexpected status code: %d", resp.StatusCode) + } + + // Limit content size to 100KB (repo page description has 100KB limit in lexicon) + limitedReader := io.LimitReader(resp.Body, 100*1024) + content, err := io.ReadAll(limitedReader) + if err != nil { + return "", fmt.Errorf("failed to read response body: %w", err) + } + + return string(content), nil +} + +// fetchAndUploadIcon fetches an image from a URL and uploads it as a blob to the user's PDS +// Returns the blob reference for use in the repo page record, or nil on error +func (s *ManifestStore) fetchAndUploadIcon(ctx context.Context, iconURL string) *atproto.ATProtoBlobRef { + // Create a context with timeout for icon fetching + fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + // Fetch the icon + req, err := http.NewRequestWithContext(fetchCtx, "GET", iconURL, nil) + if err != nil { + slog.Debug("Failed to create icon request", "url", iconURL, "error", err) + return nil + } + + req.Header.Set("User-Agent", "ATCR-Icon-Fetcher/1.0") + + client := &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 5 { + return fmt.Errorf("too many redirects") + } + return nil + }, + } + + resp, err := client.Do(req) + if err != nil { + slog.Debug("Failed to fetch icon", "url", iconURL, "error", err) + return nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + slog.Debug("Icon fetch returned non-OK status", "url", iconURL, "status", resp.StatusCode) + return nil + } + + // Validate content type - only allow images + contentType := resp.Header.Get("Content-Type") + mimeType := detectImageMimeType(contentType, iconURL) + if mimeType == "" { + slog.Debug("Icon has unsupported content type", "url", iconURL, "contentType", contentType) + return nil + } + + // Limit icon size to 3MB (matching lexicon maxSize) + limitedReader := io.LimitReader(resp.Body, 3*1024*1024) + iconData, err := io.ReadAll(limitedReader) + if err != nil { + slog.Debug("Failed to read icon data", "url", iconURL, "error", err) + return nil + } + + if len(iconData) == 0 { + slog.Debug("Icon data is empty", "url", iconURL) + return nil + } + + // Upload the icon as a blob to the user's PDS + blobRef, err := s.ctx.ATProtoClient.UploadBlob(ctx, iconData, mimeType) + if err != nil { + slog.Warn("Failed to upload icon blob", "url", iconURL, "error", err) + return nil + } + + slog.Info("Uploaded icon blob", "url", iconURL, "size", len(iconData), "mimeType", mimeType, "cid", blobRef.Ref.Link) + return blobRef +} + +// detectImageMimeType determines the MIME type for an image +// Uses Content-Type header first, then falls back to extension-based detection +// Only allows types accepted by the lexicon: image/png, image/jpeg, image/webp +func detectImageMimeType(contentType, url string) string { + // Check Content-Type header first + switch { + case strings.HasPrefix(contentType, "image/png"): + return "image/png" + case strings.HasPrefix(contentType, "image/jpeg"): + return "image/jpeg" + case strings.HasPrefix(contentType, "image/webp"): + return "image/webp" + } + + // Fall back to URL extension detection + lowerURL := strings.ToLower(url) + switch { + case strings.HasSuffix(lowerURL, ".png"): + return "image/png" + case strings.HasSuffix(lowerURL, ".jpg"), strings.HasSuffix(lowerURL, ".jpeg"): + return "image/jpeg" + case strings.HasSuffix(lowerURL, ".webp"): + return "image/webp" + } + + // Unknown or unsupported type - reject + return "" +} diff --git a/pkg/atproto/lexicon.go b/pkg/atproto/lexicon.go index 8f538e2..66f5fcf 100644 --- a/pkg/atproto/lexicon.go +++ b/pkg/atproto/lexicon.go @@ -18,9 +18,6 @@ const ( // TagCollection is the collection name for image tags TagCollection = "io.atcr.tag" - // HoldCollection is the collection name for storage holds (BYOS) - HoldCollection = "io.atcr.hold" - // HoldCrewCollection is the collection name for hold crew (membership) - LEGACY BYOS model // Stored in owner's PDS for BYOS holds HoldCrewCollection = "io.atcr.hold.crew" @@ -313,16 +310,6 @@ type HoldRecord struct { CreatedAt time.Time `json:"createdAt"` } -// NewHoldRecord creates a new hold record -func NewHoldRecord(endpoint, owner string, public bool) *HoldRecord { - return &HoldRecord{ - Type: HoldCollection, - Endpoint: endpoint, - Owner: owner, - Public: public, - CreatedAt: time.Now(), - } -} // SailorProfileRecord represents a user's profile with registry preferences // Stored in the user's PDS to configure default hold and other settings diff --git a/pkg/atproto/lexicon_test.go b/pkg/atproto/lexicon_test.go index f983cbd..45dfa45 100644 --- a/pkg/atproto/lexicon_test.go +++ b/pkg/atproto/lexicon_test.go @@ -452,56 +452,6 @@ func TestTagRecord_GetManifestDigest(t *testing.T) { } } -func TestNewHoldRecord(t *testing.T) { - tests := []struct { - name string - endpoint string - owner string - public bool - }{ - { - name: "public hold", - endpoint: "https://hold1.example.com", - owner: "did:plc:alice123", - public: true, - }, - { - name: "private hold", - endpoint: "https://hold2.example.com", - owner: "did:plc:bob456", - public: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - before := time.Now() - record := NewHoldRecord(tt.endpoint, tt.owner, tt.public) - after := time.Now() - - if record.Type != HoldCollection { - t.Errorf("Type = %v, want %v", record.Type, HoldCollection) - } - - if record.Endpoint != tt.endpoint { - t.Errorf("Endpoint = %v, want %v", record.Endpoint, tt.endpoint) - } - - if record.Owner != tt.owner { - t.Errorf("Owner = %v, want %v", record.Owner, tt.owner) - } - - if record.Public != tt.public { - t.Errorf("Public = %v, want %v", record.Public, tt.public) - } - - if record.CreatedAt.Before(before) || record.CreatedAt.After(after) { - t.Errorf("CreatedAt = %v, want between %v and %v", record.CreatedAt, before, after) - } - }) - } -} - func TestNewSailorProfileRecord(t *testing.T) { tests := []struct { name string diff --git a/pkg/auth/oauth/client.go b/pkg/auth/oauth/client.go index 22bd444..e2fe5ff 100644 --- a/pkg/auth/oauth/client.go +++ b/pkg/auth/oauth/client.go @@ -95,6 +95,7 @@ func GetDefaultScopes(did string) []string { fmt.Sprintf("repo:%s", atproto.TagCollection), fmt.Sprintf("repo:%s", atproto.StarCollection), fmt.Sprintf("repo:%s", atproto.SailorProfileCollection), + fmt.Sprintf("repo:%s", atproto.RepoPageCollection), ) return scopes