diff --git a/lexicons/io/atcr/tag.json b/lexicons/io/atcr/tag.json index 4774292..73fcf8e 100644 --- a/lexicons/io/atcr/tag.json +++ b/lexicons/io/atcr/tag.json @@ -8,7 +8,7 @@ "key": "any", "record": { "type": "object", - "required": ["repository", "tag", "manifestDigest", "createdAt"], + "required": ["repository", "tag", "createdAt"], "properties": { "repository": { "type": "string", @@ -20,9 +20,14 @@ "description": "Tag name (e.g., 'latest', 'v1.0.0', '12-slim')", "maxLength": 128 }, + "manifest": { + "type": "string", + "format": "at-uri", + "description": "AT-URI of the manifest this tag points to (e.g., 'at://did:plc:xyz/io.atcr.manifest/abc123'). Preferred over manifestDigest for new records." + }, "manifestDigest": { "type": "string", - "description": "Digest of the manifest this tag points to (e.g., 'sha256:...')" + "description": "DEPRECATED: Digest of the manifest (e.g., 'sha256:...'). Kept for backward compatibility with old records. New records should use 'manifest' field instead." }, "createdAt": { "type": "string", diff --git a/pkg/appview/jetstream/backfill.go b/pkg/appview/jetstream/backfill.go index 1dc1911..fe71e17 100644 --- a/pkg/appview/jetstream/backfill.go +++ b/pkg/appview/jetstream/backfill.go @@ -400,12 +400,18 @@ func (b *BackfillWorker) processTagRecord(did string, record *atproto.Record) er return fmt.Errorf("failed to unmarshal tag: %w", err) } + // Extract digest from tag record (tries manifest field first, falls back to manifestDigest) + manifestDigest, err := tagRecord.GetManifestDigest() + if err != nil { + return fmt.Errorf("failed to get manifest digest from tag record: %w", err) + } + // Insert or update tag return db.UpsertTag(b.db, &db.Tag{ DID: did, Repository: tagRecord.Repository, Tag: tagRecord.Tag, - Digest: tagRecord.ManifestDigest, + Digest: manifestDigest, CreatedAt: tagRecord.UpdatedAt, }) } diff --git a/pkg/appview/jetstream/worker.go b/pkg/appview/jetstream/worker.go index 09d5c1a..923be67 100644 --- a/pkg/appview/jetstream/worker.go +++ b/pkg/appview/jetstream/worker.go @@ -560,12 +560,18 @@ func (w *Worker) processTag(commit *CommitEvent) error { return nil } + // Extract digest from tag record (tries manifest field first, falls back to manifestDigest) + manifestDigest, err := tagRecord.GetManifestDigest() + if err != nil { + return fmt.Errorf("failed to get manifest digest from tag record: %w", err) + } + // Insert or update tag return db.UpsertTag(w.db, &db.Tag{ DID: commit.DID, Repository: tagRecord.Repository, Tag: tagRecord.Tag, - Digest: tagRecord.ManifestDigest, + Digest: manifestDigest, CreatedAt: tagRecord.UpdatedAt, }) } diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index 7e58b43..c80ea8b 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -310,7 +310,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name HoldDID: holdDID, PDSEndpoint: pdsEndpoint, Repository: repositoryName, - ServiceToken: serviceToken, // Cached service token from middleware validation + ServiceToken: serviceToken, // Cached service token from middleware validation ATProtoClient: atprotoClient, Database: nr.database, Authorizer: nr.authorizer, diff --git a/pkg/atproto/client.go b/pkg/atproto/client.go index 2c0e143..3973c19 100644 --- a/pkg/atproto/client.go +++ b/pkg/atproto/client.go @@ -661,3 +661,8 @@ func (c *Client) FetchDIDDocument(ctx context.Context, didDocURL string) (*DIDDo return &didDoc, nil } + +// DID returns the DID associated with this client +func (c *Client) DID() string { + return c.did +} diff --git a/pkg/atproto/lexicon.go b/pkg/atproto/lexicon.go index 2559d5c..6ad2882 100644 --- a/pkg/atproto/lexicon.go +++ b/pkg/atproto/lexicon.go @@ -241,21 +241,37 @@ type TagRecord struct { // Tag is the tag name (e.g., "latest", "v1.0.0") Tag string `json:"tag"` - // ManifestDigest is the digest of the manifest this tag points to - ManifestDigest string `json:"manifestDigest"` + // Manifest is the AT-URI of the manifest this tag points to + // Format: at://did:plc:xyz/io.atcr.manifest/abc123 + // Preferred over ManifestDigest for new records + Manifest string `json:"manifest,omitempty"` + + // ManifestDigest is the digest of the manifest this tag points to (DEPRECATED) + // Kept for backward compatibility with old records + // New records should use Manifest field instead + ManifestDigest string `json:"manifestDigest,omitempty"` // UpdatedAt timestamp UpdatedAt time.Time `json:"updatedAt"` } -// NewTagRecord creates a new tag record -func NewTagRecord(repository, tag, manifestDigest string) *TagRecord { +// NewTagRecord creates a new tag record with manifest AT-URI +// did: The DID of the user (e.g., "did:plc:xyz123") +// repository: The repository name (e.g., "myapp") +// tag: The tag name (e.g., "latest", "v1.0.0") +// manifestDigest: The manifest digest (e.g., "sha256:abc123...") +func NewTagRecord(did, repository, tag, manifestDigest string) *TagRecord { + // Build AT-URI for the manifest + // Format: at://did:plc:xyz/io.atcr.manifest/ + manifestURI := BuildManifestURI(did, manifestDigest) + return &TagRecord{ - Type: TagCollection, - Repository: repository, - Tag: tag, - ManifestDigest: manifestDigest, - UpdatedAt: time.Now(), + Type: TagCollection, + Repository: repository, + Tag: tag, + Manifest: manifestURI, + // Note: ManifestDigest is not set for new records (only for backward compat with old records) + UpdatedAt: time.Now(), } } @@ -412,6 +428,61 @@ func isDID(s string) bool { return len(s) > 4 && s[:4] == "did:" } +// BuildManifestURI creates an AT-URI for a manifest record +// did: The DID of the user (e.g., "did:plc:xyz123") +// manifestDigest: The manifest digest (e.g., "sha256:abc123...") +// Returns: AT-URI in format "at://did:plc:xyz/io.atcr.manifest/" +func BuildManifestURI(did, manifestDigest string) string { + // Remove the "sha256:" prefix from the digest to get the rkey + rkey := strings.TrimPrefix(manifestDigest, "sha256:") + return fmt.Sprintf("at://%s/%s/%s", did, ManifestCollection, rkey) +} + +// ParseManifestURI extracts the digest from a manifest AT-URI +// manifestURI: AT-URI in format "at://did:plc:xyz/io.atcr.manifest/" +// Returns: Full digest with "sha256:" prefix (e.g., "sha256:abc123...") +func ParseManifestURI(manifestURI string) (string, error) { + // Expected format: at://did:plc:xyz/io.atcr.manifest/ + if !strings.HasPrefix(manifestURI, "at://") { + return "", fmt.Errorf("invalid AT-URI format: must start with 'at://'") + } + + // Remove "at://" prefix + remainder := strings.TrimPrefix(manifestURI, "at://") + + // Split by "/" + parts := strings.Split(remainder, "/") + if len(parts) != 3 { + return "", fmt.Errorf("invalid AT-URI format: expected 3 parts (did/collection/rkey), got %d", len(parts)) + } + + // Validate collection + if parts[1] != ManifestCollection { + return "", fmt.Errorf("invalid AT-URI: expected collection %s, got %s", ManifestCollection, parts[1]) + } + + // The rkey is the digest without the "sha256:" prefix + // Add it back to get the full digest + rkey := parts[2] + return "sha256:" + rkey, nil +} + +// GetManifestDigest extracts the digest from a TagRecord, preferring the manifest field +// Returns the digest with "sha256:" prefix (e.g., "sha256:abc123...") +func (t *TagRecord) GetManifestDigest() (string, error) { + // Prefer the new manifest field + if t.Manifest != "" { + return ParseManifestURI(t.Manifest) + } + + // Fall back to the legacy manifestDigest field + if t.ManifestDigest != "" { + return t.ManifestDigest, nil + } + + return "", fmt.Errorf("tag record has neither manifest nor manifestDigest field") +} + // ============================================================================= // Embedded PDS Types (Hold Service) // ============================================================================= diff --git a/pkg/atproto/lexicon_test.go b/pkg/atproto/lexicon_test.go index 0ae88c5..78b302c 100644 --- a/pkg/atproto/lexicon_test.go +++ b/pkg/atproto/lexicon_test.go @@ -267,8 +267,9 @@ func TestNewManifestRecord(t *testing.T) { } func TestNewTagRecord(t *testing.T) { + did := "did:plc:test123" before := time.Now() - record := NewTagRecord("myapp", "latest", "sha256:abc123") + record := NewTagRecord(did, "myapp", "latest", "sha256:abc123") after := time.Now() if record.Type != TagCollection { @@ -283,8 +284,15 @@ func TestNewTagRecord(t *testing.T) { t.Errorf("Tag = %v, want latest", record.Tag) } - if record.ManifestDigest != "sha256:abc123" { - t.Errorf("ManifestDigest = %v, want sha256:abc123", record.ManifestDigest) + // New records should have manifest field (AT-URI) + expectedURI := "at://did:plc:test123/io.atcr.manifest/abc123" + if record.Manifest != expectedURI { + t.Errorf("Manifest = %v, want %v", record.Manifest, expectedURI) + } + + // New records should NOT have manifestDigest field + if record.ManifestDigest != "" { + t.Errorf("ManifestDigest should be empty for new records, got %v", record.ManifestDigest) } if record.UpdatedAt.Before(before) || record.UpdatedAt.After(after) { @@ -292,6 +300,158 @@ func TestNewTagRecord(t *testing.T) { } } +func TestBuildManifestURI(t *testing.T) { + tests := []struct { + name string + did string + manifestDigest string + want string + }{ + { + name: "standard digest", + did: "did:plc:abc123", + manifestDigest: "sha256:def456", + want: "at://did:plc:abc123/io.atcr.manifest/def456", + }, + { + name: "long digest", + did: "did:web:hold.example.com", + manifestDigest: "sha256:abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", + want: "at://did:web:hold.example.com/io.atcr.manifest/abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := BuildManifestURI(tt.did, tt.manifestDigest) + if got != tt.want { + t.Errorf("BuildManifestURI() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestParseManifestURI(t *testing.T) { + tests := []struct { + name string + manifestURI string + want string + wantErr bool + }{ + { + name: "valid URI", + manifestURI: "at://did:plc:abc123/io.atcr.manifest/def456", + want: "sha256:def456", + wantErr: false, + }, + { + name: "valid URI with did:web", + manifestURI: "at://did:web:hold.example.com/io.atcr.manifest/xyz789", + want: "sha256:xyz789", + wantErr: false, + }, + { + name: "invalid prefix", + manifestURI: "https://example.com/manifest", + want: "", + wantErr: true, + }, + { + name: "wrong collection", + manifestURI: "at://did:plc:abc123/io.atcr.tag/def456", + want: "", + wantErr: true, + }, + { + name: "too few parts", + manifestURI: "at://did:plc:abc123/io.atcr.manifest", + want: "", + wantErr: true, + }, + { + name: "too many parts", + manifestURI: "at://did:plc:abc123/io.atcr.manifest/def456/extra", + want: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseManifestURI(tt.manifestURI) + if (err != nil) != tt.wantErr { + t.Errorf("ParseManifestURI() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("ParseManifestURI() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestTagRecord_GetManifestDigest(t *testing.T) { + tests := []struct { + name string + record TagRecord + want string + wantErr bool + }{ + { + name: "new record with manifest field", + record: TagRecord{ + Manifest: "at://did:plc:test123/io.atcr.manifest/abc123", + }, + want: "sha256:abc123", + wantErr: false, + }, + { + name: "old record with manifestDigest field", + record: TagRecord{ + ManifestDigest: "sha256:def456", + }, + want: "sha256:def456", + wantErr: false, + }, + { + name: "prefers manifest over manifestDigest", + record: TagRecord{ + Manifest: "at://did:plc:test123/io.atcr.manifest/abc123", + ManifestDigest: "sha256:def456", + }, + want: "sha256:abc123", + wantErr: false, + }, + { + name: "no fields set", + record: TagRecord{}, + want: "", + wantErr: true, + }, + { + name: "invalid manifest URI", + record: TagRecord{ + Manifest: "invalid-uri", + }, + want: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.record.GetManifestDigest() + if (err != nil) != tt.wantErr { + t.Errorf("GetManifestDigest() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetManifestDigest() = %v, want %v", got, tt.want) + } + }) + } +} + func TestNewHoldRecord(t *testing.T) { tests := []struct { name string diff --git a/pkg/atproto/manifest_store.go b/pkg/atproto/manifest_store.go index d884904..e43fa47 100644 --- a/pkg/atproto/manifest_store.go +++ b/pkg/atproto/manifest_store.go @@ -184,7 +184,7 @@ func (s *ManifestStore) Put(ctx context.Context, manifest distribution.Manifest, for _, option := range options { if tagOpt, ok := option.(distribution.WithTagOption); ok { tag := tagOpt.Tag - tagRecord := NewTagRecord(s.repository, tag, dgst.String()) + tagRecord := NewTagRecord(s.client.DID(), s.repository, tag, dgst.String()) tagRKey := repositoryTagToRKey(s.repository, tag) _, err = s.client.PutRecord(ctx, TagCollection, tagRKey, tagRecord) if err != nil { diff --git a/pkg/atproto/tag_store.go b/pkg/atproto/tag_store.go index 4160cfa..4d74709 100644 --- a/pkg/atproto/tag_store.go +++ b/pkg/atproto/tag_store.go @@ -40,8 +40,14 @@ func (s *TagStore) Get(ctx context.Context, tag string) (distribution.Descriptor return distribution.Descriptor{}, fmt.Errorf("failed to unmarshal tag record: %w", err) } + // Extract manifest digest (tries manifest field first, falls back to manifestDigest) + manifestDigest, err := tagRecord.GetManifestDigest() + if err != nil { + return distribution.Descriptor{}, fmt.Errorf("failed to get manifest digest from tag record: %w", err) + } + // Parse manifest digest - dgst, err := digest.Parse(tagRecord.ManifestDigest) + dgst, err := digest.Parse(manifestDigest) if err != nil { return distribution.Descriptor{}, fmt.Errorf("invalid manifest digest in tag record: %w", err) } @@ -55,8 +61,8 @@ func (s *TagStore) Get(ctx context.Context, tag string) (distribution.Descriptor // Tag associates a tag with a descriptor (manifest digest) func (s *TagStore) Tag(ctx context.Context, tag string, desc distribution.Descriptor) error { - // Create tag record - tagRecord := NewTagRecord(s.repository, tag, desc.Digest.String()) + // Create tag record with manifest AT-URI + tagRecord := NewTagRecord(s.client.DID(), s.repository, tag, desc.Digest.String()) // Store in ATProto rkey := repositoryTagToRKey(s.repository, tag) @@ -116,7 +122,18 @@ func (s *TagStore) Lookup(ctx context.Context, desc distribution.Descriptor) ([] } // Only include tags for this repository that match the digest - if tagRecord.Repository == s.repository && tagRecord.ManifestDigest == desc.Digest.String() { + if tagRecord.Repository != s.repository { + continue + } + + // Extract digest from tag record (tries manifest field first, falls back to manifestDigest) + manifestDigest, err := tagRecord.GetManifestDigest() + if err != nil { + // Skip records with invalid manifest references + continue + } + + if manifestDigest == desc.Digest.String() { tags = append(tags, tagRecord.Tag) } } diff --git a/pkg/atproto/tag_store_test.go b/pkg/atproto/tag_store_test.go index fbefd71..24ff8f7 100644 --- a/pkg/atproto/tag_store_test.go +++ b/pkg/atproto/tag_store_test.go @@ -128,6 +128,72 @@ func TestTagStore_Get_InvalidDigest(t *testing.T) { } } +// TestTagStore_Get_BackwardCompatibility tests reading old tag records with only manifestDigest field +func TestTagStore_Get_BackwardCompatibility(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Old tag record with only manifestDigest (no manifest field) + response := `{ + "uri": "at://did:plc:test123/io.atcr.tag/myapp_latest", + "cid": "bafytest", + "value": { + "$type": "io.atcr.tag", + "repository": "myapp", + "tag": "latest", + "manifestDigest": "sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + "updatedAt": "2025-01-01T00:00:00Z" + } + }` + w.WriteHeader(http.StatusOK) + w.Write([]byte(response)) + })) + defer server.Close() + + client := NewClient(server.URL, "did:plc:test123", "test-token") + store := NewTagStore(client, "myapp") + + desc, err := store.Get(context.Background(), "latest") + if err != nil { + t.Fatalf("Get() error = %v, should handle old records", err) + } + + if desc.Digest.String() != "sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" { + t.Errorf("Digest = %v, want sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", desc.Digest.String()) + } +} + +// TestTagStore_Get_NewManifestField tests reading new tag records with manifest field +func TestTagStore_Get_NewManifestField(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // New tag record with manifest field + response := `{ + "uri": "at://did:plc:test123/io.atcr.tag/myapp_latest", + "cid": "bafytest", + "value": { + "$type": "io.atcr.tag", + "repository": "myapp", + "tag": "latest", + "manifest": "at://did:plc:test123/io.atcr.manifest/fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321", + "updatedAt": "2025-01-01T00:00:00Z" + } + }` + w.WriteHeader(http.StatusOK) + w.Write([]byte(response)) + })) + defer server.Close() + + client := NewClient(server.URL, "did:plc:test123", "test-token") + store := NewTagStore(client, "myapp") + + desc, err := store.Get(context.Background(), "latest") + if err != nil { + t.Fatalf("Get() error = %v", err) + } + + if desc.Digest.String() != "sha256:fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321" { + t.Errorf("Digest = %v, want sha256:fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321", desc.Digest.String()) + } +} + // TestTagStore_Tag tests creating/updating a tag func TestTagStore_Tag(t *testing.T) { tests := []struct { @@ -226,8 +292,14 @@ func TestTagStore_Tag(t *testing.T) { if sentTagRecord.Tag != tt.tag { t.Errorf("Tag = %v, want %v", sentTagRecord.Tag, tt.tag) } - if sentTagRecord.ManifestDigest != tt.digest.String() { - t.Errorf("ManifestDigest = %v, want %v", sentTagRecord.ManifestDigest, tt.digest.String()) + // New records should have manifest field + expectedURI := BuildManifestURI("did:plc:test123", tt.digest.String()) + if sentTagRecord.Manifest != expectedURI { + t.Errorf("Manifest = %v, want %v", sentTagRecord.Manifest, expectedURI) + } + // New records should NOT have manifestDigest field + if sentTagRecord.ManifestDigest != "" { + t.Errorf("ManifestDigest should be empty for new records, got %v", sentTagRecord.ManifestDigest) } } }) diff --git a/pkg/auth/session.go b/pkg/auth/session.go index f5b2419..33e47fb 100644 --- a/pkg/auth/session.go +++ b/pkg/auth/session.go @@ -1,6 +1,7 @@ package auth import ( + "atcr.io/pkg/atproto" "bytes" "context" "crypto/sha256" @@ -11,7 +12,6 @@ import ( "net/http" "sync" "time" - "atcr.io/pkg/atproto" "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" diff --git a/pkg/auth/token/cache.go b/pkg/auth/token/cache.go index 58e95f6..dc637fd 100644 --- a/pkg/auth/token/cache.go +++ b/pkg/auth/token/cache.go @@ -140,9 +140,9 @@ func GetCacheStats() map[string]interface{} { } return map[string]interface{}{ - "total_entries": len(globalServiceTokens), - "valid_tokens": validCount, - "expired_tokens": expiredCount, + "total_entries": len(globalServiceTokens), + "valid_tokens": validCount, + "expired_tokens": expiredCount, } } diff --git a/pkg/hold/oci/xrpc_test.go b/pkg/hold/oci/xrpc_test.go index 42b7c99..5c320bb 100644 --- a/pkg/hold/oci/xrpc_test.go +++ b/pkg/hold/oci/xrpc_test.go @@ -11,8 +11,8 @@ import ( "strconv" "testing" - "atcr.io/pkg/hold/pds" "atcr.io/pkg/atproto" + "atcr.io/pkg/hold/pds" "atcr.io/pkg/s3" "github.com/distribution/distribution/v3/registry/storage/driver/factory" _ "github.com/distribution/distribution/v3/registry/storage/driver/filesystem"