deleting a cids/ member also drops its reverse alias

internal/storage/cid_resolution.go

@@ -157,6 +157,42 @@ func (c *Client) addAliasLocked(ctx context.Context, uuid, cidStr string) error
 	return nil
 }
 
+// removeAliasLocked is the inverse of addAliasLocked: it drops the reverse-index
+// entry cid:<cid> → <uuid>. The caller must already hold c.mu, and must delete
+// the cids/<cid> key-set member itself (DeleteProperty does both).
+//
+// ⚠ Why this exists. A cids/ member write touches THREE keys — the flat
+// file:<uuid>/cids/<cid>, the __fields__ index entry, and this reverse alias —
+// while the delete path only ever removed the first two. A "deleted" CID
+// therefore still resolved: GET /meta/<cid>, ResolveRoot and GetByCID all kept
+// answering with the record. The asymmetry is not theoretical. meta-share
+// published dag-pb roots computed over RAR volumes and par2 files onto six
+// records on watch.nsl.sh; removing the member was expressible, removing the
+// claim was not, so the false identity survived its own retraction.
+//
+// Guarded, not blind: the alias is removed only when it currently points at
+// THIS uuid. Two records can legitimately race to claim a cid (see the
+// dual-root guard in addAliasLocked, which leaves the first writer's alias in
+// place); deleting a member from the loser must not unpick the winner's index.
+func (c *Client) removeAliasLocked(ctx context.Context, uuid, cidStr string) error {
+	if uuid == "" || cidStr == "" {
+		return nil
+	}
+	indexKey := c.buildCIDIndexKey(cidStr)
+	existing, err := c.client.Get(ctx, indexKey).Result()
+	if err == redis.Nil {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("read existing alias: %w", err)
+	}
+	if existing != uuid {
+		// Somebody else owns this alias. Leave it.
+		return nil
+	}
+	return c.client.Del(ctx, indexKey).Err()
+}
+
 // ResolveRoot maps a hash supplied by a caller (a meta-sort write, an editor
 // lookup, an SSE consumer) to the actual storage root key.
 //

internal/storage/cid_retraction_test.go

@@ -0,0 +1,158 @@
+package storage
+
+import (
+	"context"
+	"testing"
+)
+
+// Tests for retracting a `cids/<cid>` key-set member (cid_resolution.go's
+// removeAliasLocked, reached through DeleteProperty).
+//
+// Why this is worth its own file. Adding a member writes THREE keys — the flat
+// `file:<uuid>/cids/<cid>`, the `__fields__` index entry, and the reverse alias
+// `cid:<cid>` → `<uuid>`. Until removeAliasLocked existed, deleting removed only
+// the first two, so a retracted CID kept resolving to the record: the member was
+// gone from the document and the claim was not.
+//
+// That asymmetry had a live consequence. meta-share minted dag-pb roots and
+// midhash256s over whatever `archive::pick_media` elected, which on an
+// obfuscated posting was an arbitrary RAR volume or a par2 file. Six records on
+// watch.nsl.sh carry such members. `METADATA_KEYS.md` §2 defines a `cids/`
+// member as a digest of *this record's file* and notes that every member is a
+// reverse index back to the record — so those entries assert that an archive
+// volume is the episode. Retracting one has to remove the assertion, not just
+// the row.
+
+// aliasTarget returns the uuid that cid:<cidStr> currently resolves to, or "".
+func aliasTarget(t *testing.T, c *Client, cidStr string) string {
+	t.Helper()
+	got, err := c.client.Get(context.Background(), c.buildCIDIndexKey(cidStr)).Result()
+	if err != nil {
+		return ""
+	}
+	return got
+}
+
+func hasField(t *testing.T, c *Client, hashID, field string) bool {
+	t.Helper()
+	ok, err := c.client.SIsMember(context.Background(), c.buildFieldsKey(hashID), field).Result()
+	if err != nil {
+		t.Fatalf("sismember: %v", err)
+	}
+	return ok
+}
+
+// The real shape: `…glnrqztt…` is the nzb-release locator whose record published
+// a dag-pb root computed over volume 3 of a 35-volume RAR5 set.
+const (
+	poisonedRoot = "bafybeicicuzdvuztca2hp2g2k7scgr4nnibilzgoppljeprphybqot7v6i"
+	goodMidhash  = "bagacbabaed6uiulnv7tscpxne22eztkjvnxg5almn43dpfecppixarfe7xhlu"
+)
+
+func TestRetractingACidMemberRemovesTheClaimNotJustTheRow(t *testing.T) {
+	forEachPrefix(t, func(t *testing.T, c *Client) {
+		const uuid = "01KSBHMTESTRECORD"
+
+		if err := c.SetMetadataFlat(uuid, map[string]string{
+			"cids/" + poisonedRoot: "true",
+			"cids/" + goodMidhash:  "true",
+			"fileType":             "video",
+		}); err != nil {
+			t.Fatalf("seed: %v", err)
+		}
+
+		// All three writes landed, and the CID resolves to the record.
+		if got := aliasTarget(t, c, poisonedRoot); got != uuid {
+			t.Fatalf("alias should point at the record before retraction, got %q", got)
+		}
+		if !hasField(t, c, uuid, "cids/"+poisonedRoot) {
+			t.Fatal("field index should list the member before retraction")
+		}
+		if got := c.ResolveRoot(poisonedRoot); got != uuid {
+			t.Fatalf("ResolveRoot should find the record before retraction, got %q", got)
+		}
+
+		if err := c.DeleteProperty(uuid, "cids/"+poisonedRoot); err != nil {
+			t.Fatalf("DeleteProperty: %v", err)
+		}
+
+		// 1. the flat key
+		if v, err := c.client.Get(context.Background(),
+			c.buildKeyPrefix(uuid)+"cids/"+poisonedRoot).Result(); err == nil {
+			t.Fatalf("flat key should be gone, still %q", v)
+		}
+		// 2. the field index
+		if hasField(t, c, uuid, "cids/"+poisonedRoot) {
+			t.Fatal("field index still lists the retracted member")
+		}
+		// 3. the reverse alias — the one that used to survive
+		if got := aliasTarget(t, c, poisonedRoot); got != "" {
+			t.Fatalf("reverse alias survived retraction, still points at %q", got)
+		}
+		if got := c.ResolveRoot(poisonedRoot); got == uuid {
+			t.Fatal("a retracted cid must not resolve to the record any more")
+		}
+
+		// The siblings are untouched: retraction is surgical, not a purge.
+		if got := aliasTarget(t, c, goodMidhash); got != uuid {
+			t.Fatalf("an unrelated sibling lost its alias, got %q", got)
+		}
+		if !hasField(t, c, uuid, "cids/"+goodMidhash) {
+			t.Fatal("an unrelated sibling lost its field-index entry")
+		}
+		if v, err := c.GetProperty(uuid, "fileType"); err != nil || v != "video" {
+			t.Fatalf("an unrelated field changed: %q %v", v, err)
+		}
+	})
+}
+
+// Two records can race to claim one cid; addAliasLocked leaves the first
+// writer's alias in place. Retracting the loser's member must not unpick the
+// winner's index — which is why removeAliasLocked compares before deleting.
+func TestRetractingDoesNotStealAnotherRecordsAlias(t *testing.T) {
+	forEachPrefix(t, func(t *testing.T, c *Client) {
+		const winner = "01KSBHMWINNER"
+		const loser = "01KSBHMLOSER"
+		const shared = "bafybeiaeb2lqi4z3np3q4tarldtyjp2t5lvnoodmtpvpi2tz73azn5td2m"
+
+		if err := c.SetMetadataFlat(winner, map[string]string{"cids/" + shared: "true"}); err != nil {
+			t.Fatalf("seed winner: %v", err)
+		}
+		if err := c.SetMetadataFlat(loser, map[string]string{"cids/" + shared: "true"}); err != nil {
+			t.Fatalf("seed loser: %v", err)
+		}
+		if got := aliasTarget(t, c, shared); got != winner {
+			t.Fatalf("the first writer should own the alias, got %q", got)
+		}
+
+		if err := c.DeleteProperty(loser, "cids/"+shared); err != nil {
+			t.Fatalf("DeleteProperty: %v", err)
+		}
+
+		if got := aliasTarget(t, c, shared); got != winner {
+			t.Fatalf("retracting the loser's member stole the winner's alias, now %q", got)
+		}
+		if hasField(t, c, loser, "cids/"+shared) {
+			t.Fatal("the loser's own member should still be gone")
+		}
+	})
+}
+
+// A non-cids field has no alias to clean up, and the delete must not invent one.
+func TestRetractingAnOrdinaryFieldTouchesNoAlias(t *testing.T) {
+	forEachPrefix(t, func(t *testing.T, c *Client) {
+		const uuid = "01KSBHMORDINARY"
+		if err := c.SetMetadataFlat(uuid, map[string]string{
+			"cids/" + goodMidhash: "true",
+			"quality":             "1080p",
+		}); err != nil {
+			t.Fatalf("seed: %v", err)
+		}
+		if err := c.DeleteProperty(uuid, "quality"); err != nil {
+			t.Fatalf("DeleteProperty: %v", err)
+		}
+		if got := aliasTarget(t, c, goodMidhash); got != uuid {
+			t.Fatalf("deleting an ordinary field disturbed a cid alias, got %q", got)
+		}
+	})
+}

internal/storage/client.go

@@ -460,6 +460,10 @@ func (c *Client) MergeMetadataFlat(hashID string, metadata map[string]string) (i
 
 // DeleteProperty deletes a single property
 // Uses flat key: DEL file:{hashId}/{property}
+//
+// For a cids/<cid> member this is the full inverse of the write: the flat key,
+// the __fields__ entry AND the cid:<cid> reverse alias all go. Anything less
+// leaves the CID still resolving to this record — see removeAliasLocked.
 func (c *Client) DeleteProperty(hashID, property string) error {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
@@ -475,6 +479,13 @@ func (c *Client) DeleteProperty(hashID, property string) error {
 	if err := c.client.Del(ctx, key).Err(); err != nil {
 		return err
 	}
+	// Before the field index forgets the name, use it: a cids/ member owns a
+	// reverse-index entry that nothing else will ever clean up.
+	if cidStr := cidFromKeysetField(property); cidStr != "" {
+		if err := c.removeAliasLocked(ctx, hashID, cidStr); err != nil {
+			return fmt.Errorf("remove alias for %s: %w", cidStr, err)
+		}
+	}
 	c.removeFieldLocked(ctx, hashID, property)
 	c.markDirtyLocked(ctx, hashID)
 	return nil