From ecda86f1e0b4a456ea1f4ada02acd9df778830fd Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 07:33:11 -0700 Subject: [PATCH 1/6] =?UTF-8?q?test(posts):=20RED=20=E2=80=94=20read-path?= =?UTF-8?q?=20visibility=20predicate=20(task=207=20cycle=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compensating control for task 6's write-path flip: a pending/rejected/ removed post must be UNREACHABLE through every read path for a non-author. - T2 binding contract (tests/e2e/read_visibility_contract_test.go): accepted post reachable on community feed + discover + post.get + getComments; pending invisible on all; removed renders as #removedPost + code. Public (anonymous) viewer — the author-with-status arc is T1 (no sealed session at T2). - T1 predicate matrix (internal/db/postgres/post_visibility_test.go): the 5 display sites + user post_count. Fork-case join key pinned hardest (discover must key on a.community_did = p.community_did, or one community's acceptance publishes another's pending post). Unknown-author LEFT-JOIN + author-media-on-visible-row covered. - T0 lexicon shape (tests/lexicon_removedpost_test.go): #removedPost union member + defs, postView optional status/acceptanceUri (additive-optional). - Stub: visiblePostsJoin (the centralized predicate GREEN wires in). Co-Authored-By: Claude Fable 5 --- internal/db/postgres/post_visibility.go | 37 ++ .../postgres/post_visibility_fixtures_test.go | 177 ++++++++ internal/db/postgres/post_visibility_test.go | 393 ++++++++++++++++++ tests/e2e/read_visibility_contract_test.go | 227 ++++++++++ tests/lexicon_removedpost_test.go | 163 ++++++++ 5 files changed, 997 insertions(+) create mode 100644 internal/db/postgres/post_visibility.go create mode 100644 internal/db/postgres/post_visibility_fixtures_test.go create mode 100644 internal/db/postgres/post_visibility_test.go create mode 100644 tests/e2e/read_visibility_contract_test.go create mode 100644 tests/lexicon_removedpost_test.go diff --git a/internal/db/postgres/post_visibility.go b/internal/db/postgres/post_visibility.go new file mode 100644 index 0000000..6168b33 --- /dev/null +++ b/internal/db/postgres/post_visibility.go @@ -0,0 +1,37 @@ +package postgres + +// The centralized read-path visibility predicate (task 7, PRD §6.2). +// +// STUB — signature only. This is the single admission-aware join every posts +// display query must go through so that no read path can forget the gate (the +// piecemeal-predicate failure PRD §6.2 calls out). GREEN fills in the body and +// wires it into GetViewsByURIs, the three feed queries, GetByAuthor and the +// profile count; the visibility suites in post_visibility_test.go are red until +// it does. +// +// THE JOIN KEY IS (a.community_did = p.community_did AND a.post_uri = p.uri). +// Both halves are load-bearing. The post_uri half selects the subject; the +// community_did half is what makes a post visible iff ITS OWN community accepted +// it, which is the fork-case security property TestDiscoverVisibility_ForkJoinKey +// pins — a join on post_uri alone would let one community's acceptance publish +// another community's pending post. +// +// THE STATUS RULE depends on the viewer: +// - a non-author (viewerDID == "" or != the post's author): status = 'accepted' +// only. +// - the author of the post (viewerDID == posts.author_did): accepted OR the +// author's own non-accepted rows, so a client can render 'pending' / 'removed' +// on the author's own profile. +// +// visiblePostsJoin returns the SQL fragment to splice after the posts `p` +// reference (a JOIN plus its WHERE contribution) and the arguments it binds, +// starting at paramOffset. Returning the empty fragment — the stub's behavior — +// applies NO gate, which is the pre-task-7 state every visibility suite fails +// against. +func visiblePostsJoin(viewerDID string, paramOffset int) (sqlFragment string, args []interface{}) { + return "", nil +} + +// Referenced so the stub is not flagged as dead before GREEN wires it into the +// read queries. Delete this line once visiblePostsJoin has a real caller. +var _ = visiblePostsJoin diff --git a/internal/db/postgres/post_visibility_fixtures_test.go b/internal/db/postgres/post_visibility_fixtures_test.go new file mode 100644 index 0000000..a9f52a0 --- /dev/null +++ b/internal/db/postgres/post_visibility_fixtures_test.go @@ -0,0 +1,177 @@ +//go:build integration + +package postgres + +import ( + "context" + "database/sql" + "fmt" + "testing" + "time" + + "Coves/internal/core/posts" + + _ "github.com/lib/pq" + "github.com/stretchr/testify/require" +) + +// Shared seeding for the read-path visibility suites (task 7, PRD §6.2). +// +// Every posts read path — the three feeds, post.get, actor.getPosts, the +// comment header, the profile counts — has to answer the SAME question before +// it renders a row: has THIS community admitted THIS post? Under author-owned +// posts (PRD §2, §6.1) that answer is not a column on `posts`; it is a row in +// community_post_admissions keyed by (community_did, post_uri), and a post can +// hold independent decisions from several communities at once. So a read path +// that does not join that table shows speech no community agreed to carry. +// +// These helpers seed the two halves directly — a postv2 content row in `posts`, +// and an admission decision in community_post_admissions — the way the firehose +// consumers would once the acceptance/removal engine has run. Direct SQL rather +// than the AdmissionRepository writers or the post consumer, for the reason the +// block-filter fixtures give one file over: what is under test is the READING +// query, and driving the writers would make a visibility suite fail whenever the +// admission state machine breaks, which is a different suite's job. + +// postV2URI renders the AT-URI a postv2 record has once committed: the AUTHOR's +// DID is the authority (PRD §3.1), never the community's. The read paths must +// carry both kinds of post through one table, so the collection in the URI is +// the only thing that says which repo a row came from (see blobOwnerOf). +func postV2URI(authorDID, rkey string) string { + return "at://" + authorDID + "/" + posts.PostV2Collection + "/" + rkey +} + +// seedVisibilityPost inserts one author-owned postv2 content row and returns its +// URI. community_did is the post's INITIAL submission target (PRD §6.1) — the +// admission decision lives in a separate row, seeded with seedAdmission, so a +// post can be pending in one community and accepted in another (the fork case). +// +// The author row is NOT created here: some suites deliberately seed a post whose +// author has no `users` row (a federated author the AppView has not hydrated, +// PRD §5.3), and an INNER JOIN to users would make that post invisible even once +// its community accepts it. Callers that want a known author call createTestUser +// first. +func seedVisibilityPost(t *testing.T, db *sql.DB, communityDID, authorDID, rkey, title string, createdAt time.Time) string { + t.Helper() + + uri := postV2URI(authorDID, rkey) + _, err := db.ExecContext(context.Background(), ` + INSERT INTO posts (uri, cid, rkey, author_did, community_did, title, created_at, score, upvote_count, downvote_count) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, 0) + `, uri, "bafypostv2"+rkey, rkey, authorDID, communityDID, title, createdAt, 1, 1) + require.NoErrorf(t, err, "seeding postv2 %s", rkey) + return uri +} + +// seedVisibilityPostWithEmbed is seedVisibilityPost carrying an external embed +// with a blob thumbnail, so a suite can prove the visible row still hydrates its +// media out of the AUTHOR's repository (blobOwnerOf, PRD §3.1) after the +// admission predicate lands. A predicate that dropped the author's pds_url from +// the SELECT would address the blob to an empty host — a broken image that looks +// fine server-side. +func seedVisibilityPostWithEmbed(t *testing.T, db *sql.DB, communityDID, authorDID, rkey, title, thumbCID string, createdAt time.Time) string { + t.Helper() + + uri := postV2URI(authorDID, rkey) + embedJSON := fmt.Sprintf(`{ + "$type": "social.coves.embed.external", + "external": { + "uri": "https://example.com/article", + "title": "Example Article", + "description": "A test article", + "thumb": {"$type": "blob", "ref": {"$link": "%s"}, "mimeType": "image/jpeg", "size": 52813} + } + }`, thumbCID) + _, err := db.ExecContext(context.Background(), ` + INSERT INTO posts (uri, cid, rkey, author_did, community_did, title, embed, created_at, score, upvote_count, downvote_count) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, 0) + `, uri, "bafypostv2"+rkey, rkey, authorDID, communityDID, title, embedJSON, createdAt, 1, 1) + require.NoErrorf(t, err, "seeding postv2-with-embed %s", rkey) + return uri +} + +// seedAdmission writes one community's decision about one post directly into +// community_post_admissions. +// +// The columns set per status mirror what the admission repository leaves behind: +// an accepted row pins the acceptance record and the accepted CID; a removed or +// rejected row carries the decision code the schema's CHECK constraint requires; +// a community event (accept/remove) advances the (rev, op_rank) watermark, while +// a pending observation and a local rejection do not. Callers that only care +// about the STATUS a reader keys off can ignore the detail and pass "". +func seedVisibilityAdmission(t *testing.T, db *sql.DB, communityDID, postURI string, status posts.AdmissionStatus, acceptedCID, decisionCode string) { + t.Helper() + + var ( + acceptanceURI, acceptanceRkey, accCID, evalCID, code, rev sql.NullString + decisionAt sql.NullTime + opRank sql.NullInt16 + ) + switch status { + case posts.AdmissionStatusAccepted, posts.AdmissionStatusPendingReacceptance: + acceptanceURI = sql.NullString{String: "at://" + communityDID + "/" + posts.AcceptanceCollection + "/acc" + postURI[len(postURI)-6:], Valid: true} + acceptanceRkey = sql.NullString{String: "acc" + postURI[len(postURI)-6:], Valid: true} + if acceptedCID == "" { + acceptedCID = "bafyaccepted" + } + accCID = sql.NullString{String: acceptedCID, Valid: true} + evalCID = sql.NullString{String: acceptedCID, Valid: true} + rev = sql.NullString{String: "3lqqqqqqqqqq2", Valid: true} + opRank = sql.NullInt16{Int16: int16(posts.CommunityOpPut), Valid: true} + case posts.AdmissionStatusRemoved: + if decisionCode == "" { + decisionCode = "rule-violation" + } + code = sql.NullString{String: decisionCode, Valid: true} + decisionAt = sql.NullTime{Time: time.Now(), Valid: true} + rev = sql.NullString{String: "3lqqqqqqqqqq3", Valid: true} + opRank = sql.NullInt16{Int16: int16(posts.CommunityOpPut), Valid: true} + case posts.AdmissionStatusRejected: + if decisionCode == "" { + decisionCode = "rate-limit-exceeded" + } + code = sql.NullString{String: decisionCode, Valid: true} + decisionAt = sql.NullTime{Time: time.Now(), Valid: true} + case posts.AdmissionStatusPending: + // nothing set beyond status + default: + t.Fatalf("seedVisibilityAdmission: unknown status %q", status) + } + + _, err := db.ExecContext(context.Background(), ` + INSERT INTO community_post_admissions ( + community_did, post_uri, status, + acceptance_uri, acceptance_rkey, accepted_cid, evaluated_cid, + decision_code, decision_at, + last_community_rev, last_community_op_rank, + created_at, updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, NOW(), NOW()) + ON CONFLICT (community_did, post_uri) DO UPDATE SET + status = excluded.status, + acceptance_uri = excluded.acceptance_uri, + acceptance_rkey = excluded.acceptance_rkey, + accepted_cid = excluded.accepted_cid, + evaluated_cid = excluded.evaluated_cid, + decision_code = excluded.decision_code, + decision_at = excluded.decision_at, + last_community_rev = excluded.last_community_rev, + last_community_op_rank = excluded.last_community_op_rank, + updated_at = NOW() + `, communityDID, postURI, string(status), + acceptanceURI, acceptanceRkey, accCID, evalCID, + code, decisionAt, rev, opRank) + require.NoErrorf(t, err, "seeding admission %s for %s in %s", status, postURI, communityDID) +} + +// visibilityCommunity creates a community row plus its owner, returning the DID. +// A thin wrapper over createTestCommunity that mints the owner too, so a suite +// can stand up two communities (the fork case) without hand-rolling owners. +func visibilityCommunity(t *testing.T, db *sql.DB, label string) string { + t.Helper() + + ownerDID := "did:plc:vis" + label + "owner" + createTestUser(t, db, "vis"+label+"owner.test", ownerDID) + communityDID := "did:plc:vis" + label + "community" + createTestCommunity(t, db, communityDID, "vis"+label+".coves.social", ownerDID) + return communityDID +} diff --git a/internal/db/postgres/post_visibility_test.go b/internal/db/postgres/post_visibility_test.go new file mode 100644 index 0000000..6e85edb --- /dev/null +++ b/internal/db/postgres/post_visibility_test.go @@ -0,0 +1,393 @@ +//go:build integration + +package postgres + +import ( + "context" + "testing" + "time" + + "Coves/internal/core/communityFeeds" + "Coves/internal/core/discover" + "Coves/internal/core/posts" + "Coves/internal/core/timeline" + "Coves/tests/testkit" + + _ "github.com/lib/pq" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The read-path visibility predicate (task 7, PRD §6.2). Under author-owned +// posts a post record is indexed the moment its author writes it, but it is only +// VISIBLE in a community once that community has admitted it (PRD §2). Admission +// state is per-(community, post) in community_post_admissions, and the security +// property this whole task exists to establish is: +// +// a non-author must not reach a pending / rejected / removed post through ANY +// read path — the feeds, post.get, actor.getPosts, the comment header, or the +// counts. +// +// On this branch the read queries reference community_post_admissions NOWHERE +// (loop_state task-6 note: "post_repo.go + all feed queries currently reference +// it NOWHERE outside getStatus"), so every one of these suites fails until the +// centralized predicate lands. They are the compensating control for task 6's +// deploy gate, not a feature nicety: task 6 shipped the write-path flip, which +// means any authenticated user can already index a postv2 naming any community, +// and only this predicate stops it rendering as that community's content. + +const ( + // visibilitySort keeps every feed read chronological so the seeded set is + // the whole answer — a hot rank computed against NOW() would make an + // exact-set assertion flaky for reasons that have nothing to do with + // admission. + visibilitySort = "new" + // A public (unauthenticated) read: no viewer DID. This is the security case + // — the anonymous internet must see accepted content only. + publicViewer = "" +) + +// feedURIs renders a community feed as the URIs it returned. +func feedURIs(feed []*communityFeeds.FeedViewPost) []string { + uris := make([]string, 0, len(feed)) + for _, item := range feed { + if item.Post != nil { + uris = append(uris, item.Post.URI) + } + } + return uris +} + +func discoverFeedURIs(feed []*discover.FeedViewPost) []string { + uris := make([]string, 0, len(feed)) + for _, item := range feed { + if item.Post != nil { + uris = append(uris, item.Post.URI) + } + } + return uris +} + +func timelineFeedURIs(feed []*timeline.FeedViewPost) []string { + uris := make([]string, 0, len(feed)) + for _, item := range feed { + if item.Post != nil { + uris = append(uris, item.Post.URI) + } + } + return uris +} + +// TestCommunityFeedVisibility_AcceptedOnly is the community feed's predicate: +// getCommunity(X) shows a post iff community X has ADMITTED it. The four seeded +// posts span every admission state so the accepted one is the only survivor — +// and the pending, rejected and removed ones each prove a distinct leak the +// predicate closes. +func TestCommunityFeedVisibility_AcceptedOnly(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "cf") + author := "did:plc:viscfauthor" + createTestUser(t, db, "viscfauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "cfacc", "accepted", base.Add(4*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "cfpen", "pending", base.Add(3*time.Hour)) + rejected := seedVisibilityPost(t, db, community, author, "cfrej", "rejected", base.Add(2*time.Hour)) + removed := seedVisibilityPost(t, db, community, author, "cfrem", "removed", base.Add(1*time.Hour)) + + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2cfacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, rejected, posts.AdmissionStatusRejected, "", "spam") + seedVisibilityAdmission(t, db, community, removed, posts.AdmissionStatusRemoved, "", "rule-violation") + + repo := NewCommunityFeedRepository(db, "test-secret") + feed, _, err := repo.GetCommunityFeed(ctx, communityFeeds.GetCommunityFeedRequest{ + Community: community, ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + + assert.ElementsMatch(t, []string{accepted}, feedURIs(feed), + "a community feed must serve accepted posts only. A pending post is speech the community has not agreed to carry, "+ + "a rejected one it refused, and a removed one it took down — every non-accepted URI here is a leak the "+ + "admission predicate exists to close") +} + +// TestDiscoverVisibility_ForkJoinKey is the join-key security assertion, pinned +// hardest per the task brief. Discover spans every community, so it cannot lean +// on a `p.community_did = $1` filter the way getCommunity does — it must join the +// admission row on BOTH halves of the subject key +// (a.community_did = p.community_did AND a.post_uri = p.uri) and show a post iff +// its OWN community accepted it. +// +// The fork case is what makes the community half of that key load-bearing. One +// post carries TWO admission rows: pending in its own community B, and accepted +// in a DIFFERENT community A that forked it (PRD §2, §6.1). A predicate that +// joined on post_uri + status alone — dropping the community_did equality — would +// see the accepted (A, post) row and leak the post into discover under community +// B, which has not accepted it. That is the single most dangerous read-path bug +// this task can ship: a moderator's decision in one community silently +// publishing a post in another. +func TestDiscoverVisibility_ForkJoinKey(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + forker := visibilityCommunity(t, db, "dfA") // community A: forks/accepts the post + homeComm := visibilityCommunity(t, db, "dfB") // community B: the post's own community, still pending + author := "did:plc:visdfauthor" + createTestUser(t, db, "visdfauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + + // The forked post: its OWN community is B (pending), but community A holds an + // accepted admission for the very same URI. + forked := seedVisibilityPost(t, db, homeComm, author, "dffork", "forked-post", base.Add(3*time.Hour)) + seedVisibilityAdmission(t, db, homeComm, forked, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, forker, forked, posts.AdmissionStatusAccepted, "bafypostv2dffork", "") + + // A positive control: a post whose own community A accepted it must appear. + homegrown := seedVisibilityPost(t, db, forker, author, "dfhome", "homegrown", base.Add(2*time.Hour)) + seedVisibilityAdmission(t, db, forker, homegrown, posts.AdmissionStatusAccepted, "bafypostv2dfhome", "") + + repo := NewDiscoverRepository(db, "test-secret") + feed, _, err := repo.GetDiscover(ctx, discover.GetDiscoverRequest{ + ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + + got := discoverFeedURIs(feed) + assert.Contains(t, got, homegrown, "a post its own community accepted must appear in discover") + assert.NotContainsf(t, got, forked, + "the forked post leaked into discover. It is PENDING in its own community (%s) and only ACCEPTED in a "+ + "community that forked it (%s); the discover join must key on (a.community_did = p.community_did AND "+ + "a.post_uri = p.uri), so that a post is visible iff ITS community accepted it. A join on post_uri + status "+ + "alone lets one community's acceptance publish another community's pending post.", + homeComm, forker) +} + +// TestTimelineVisibility_AcceptedOnly is the subscribed-feed predicate. The +// timeline joins community_subscriptions, so a leak here reaches a subscriber's +// home feed directly. The seeded post is pending in the subscribed community and +// must not appear, while the accepted one must. +func TestTimelineVisibility_AcceptedOnly(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "tl") + author := "did:plc:vistlauthor" + createTestUser(t, db, "vistlauthor.test", author) + subscriber := "did:plc:vistlsubscriber" + createTestUser(t, db, "vistlsubscriber.test", subscriber) + + _, err := db.ExecContext(ctx, ` + INSERT INTO community_subscriptions (user_did, community_did, subscribed_at) + VALUES ($1, $2, NOW()) + `, subscriber, community) + require.NoError(t, err) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "tlacc", "accepted", base.Add(2*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "tlpen", "pending", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2tlacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + + repo := NewTimelineRepository(db, "test-secret") + feed, _, err := repo.GetTimeline(ctx, timeline.GetTimelineRequest{ + UserDID: subscriber, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + + assert.ElementsMatch(t, []string{accepted}, timelineFeedURIs(feed), + "a subscriber's timeline must carry accepted posts only; a pending post reaching the home feed is the "+ + "loudest possible leak of unadmitted content") +} + +// TestPostGetVisibility_PublicSeesAcceptedOnly is post.get's predicate for the +// anonymous caller. GetViewsByURIs backs social.coves.community.post.get, and a +// post absent from the returned map becomes a notFoundPost on the wire — which is +// exactly the right answer for a public caller asking about a post no community +// has admitted. +// +// This suite also pins the TWO shapes the predicate must preserve while it adds +// the admission gate: a post by an author with no `users` row (a federated +// author, PRD §5.3) stays visible once accepted, and the visible row still +// hydrates its media out of the author's repository. +func TestPostGetVisibility_PublicSeesAcceptedOnly(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "pg") + author := "did:plc:vispgauthor" + createTestUser(t, db, "vispgauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "pgacc", "accepted", base.Add(4*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "pgpen", "pending", base.Add(3*time.Hour)) + rejected := seedVisibilityPost(t, db, community, author, "pgrej", "rejected", base.Add(2*time.Hour)) + removed := seedVisibilityPost(t, db, community, author, "pgrem", "removed", base.Add(1*time.Hour)) + + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2pgacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, rejected, posts.AdmissionStatusRejected, "", "spam") + seedVisibilityAdmission(t, db, community, removed, posts.AdmissionStatusRemoved, "", "rule-violation") + + repo := NewPostRepository(db) + views, err := repo.GetViewsByURIs(ctx, []string{accepted, pending, rejected, removed}) + require.NoError(t, err) + + require.Containsf(t, views, accepted, "an accepted post must be served by post.get") + assert.NotContainsf(t, views, pending, + "post.get served a PENDING post to an anonymous caller. Absence from this map is what makes the endpoint "+ + "answer notFoundPost; a pending post that resolves here is reachable by permalink regardless of any feed gate") + assert.NotContains(t, views, rejected, "post.get must not serve a rejected post to the public") + assert.NotContains(t, views, removed, "post.get must not serve a removed post to the public") + + t.Run("an accepted post by an unindexed author is still visible", func(t *testing.T) { + // The write-path flip (PRD §5.3) drops the posts→users FK so a federated + // author with no `users` row can be indexed. The read join must be a LEFT + // join, or the whole promise of open federated posting breaks silently at + // the read path — the post indexes fine and is invisible forever (the + // loop_state task-2 → task-7 obligation). + unknownAuthor := "did:plc:visunknownfederated" + unknownPost := seedVisibilityPost(t, db, community, unknownAuthor, "pgunk", "federated", base.Add(5*time.Hour)) + seedVisibilityAdmission(t, db, community, unknownPost, posts.AdmissionStatusAccepted, "bafypostv2pgunk", "") + + got, err := repo.GetViewsByURIs(ctx, []string{unknownPost}) + require.NoError(t, err) + require.Containsf(t, got, unknownPost, + "an accepted post whose author has no users row was dropped by post.get. The author join must be a LEFT "+ + "join (author_did has no FK since migration 034); an INNER join makes every federated author's post "+ + "invisible the moment it is accepted") + view := got[unknownPost] + require.NotNil(t, view.Author) + assert.Equal(t, unknownAuthor, view.Author.DID, + "the unhydrated author's DID must still be carried — it is the repo the postv2 record lives in") + }) +} + +// TestPostGetVisibility_AuthorMediaResolvesOnVisibleRow pins that the admission +// predicate does not cost the visible row its media owner. A postv2 post's blobs +// live in the AUTHOR's repository (blobOwnerOf, PRD §3.1), so the row must carry +// the author's pds_url out of the SELECT. A predicate that narrowed the SELECT +// list, or dropped the author join to a subquery, would blank the pds_url and +// address every accepted post's image to an empty host. +// +// Not parallel: it sets the process-wide image-URL config scanPostView reads, +// the same constraint TestGetCommunityFeed_BlobURLTransformation documents. +func TestPostGetVisibility_AuthorMediaResolvesOnVisibleRow(t *testing.T) { + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "pm") + author := "did:plc:vispmauthor" + createTestUser(t, db, "vispmauthor.test", author) + + const thumbCID = "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm" + accepted := seedVisibilityPostWithEmbed(t, db, community, author, "pmacc", "with media", thumbCID, + time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2pmacc", "") + + repo := NewPostRepository(db) + views, err := repo.GetViewsByURIs(ctx, []string{accepted}) + require.NoError(t, err) + require.Contains(t, views, accepted) + + view := views[accepted] + require.NotNil(t, view.Author) + assert.NotEmptyf(t, view.Author.PDSURL, + "the visible accepted postv2 row lost its author pds_url. A postv2 post's blobs live in the author's repo, "+ + "so the visibility predicate must keep selecting author pds_url; blanking it addresses the post's media to "+ + "an empty host — a broken image that looks fine server-side (blobOwnerOf, PRD §3.1)") +} + +// TestActorPostsVisibility_AuthorVsNonAuthor is actor.getPosts, the one display +// surface that already threads a viewer DID (GetAuthorPostsRequest.ViewerDID). +// That makes it the site where BOTH halves of the predicate are testable today: +// a non-author sees the author's accepted posts only, while the author sees +// their own pending and removed posts too (PRD §6.2 — "authors see their own +// posts with per-community status"). +func TestActorPostsVisibility_AuthorVsNonAuthor(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "ap") + author := "did:plc:visapauthor" + createTestUser(t, db, "visapauthor.test", author) + stranger := "did:plc:visapstranger" + createTestUser(t, db, "visapstranger.test", stranger) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "apacc", "accepted", base.Add(3*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "appen", "pending", base.Add(2*time.Hour)) + removed := seedVisibilityPost(t, db, community, author, "aprem", "removed", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2apacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, removed, posts.AdmissionStatusRemoved, "", "rule-violation") + + repo := NewPostRepository(db) + read := func(t *testing.T, viewerDID string) []string { + t.Helper() + views, _, err := repo.GetByAuthor(ctx, posts.GetAuthorPostsRequest{ + ActorDID: author, ViewerDID: viewerDID, Limit: 50, + }) + require.NoError(t, err) + uris := make([]string, 0, len(views)) + for _, v := range views { + uris = append(uris, v.URI) + } + return uris + } + + t.Run("a stranger sees the author's accepted posts only", func(t *testing.T) { + assert.ElementsMatch(t, []string{accepted}, read(t, stranger), + "actor.getPosts served a non-author the author's pending and removed posts. This is the alternate-endpoint "+ + "leak the task brief names: the feed gate is worthless if the author-feed shows the same content ungated") + }) + + t.Run("a stranger is the same as the anonymous public", func(t *testing.T) { + assert.ElementsMatch(t, []string{accepted}, read(t, publicViewer)) + }) + + t.Run("the author sees their own non-accepted posts", func(t *testing.T) { + assert.ElementsMatch(t, []string{accepted, pending, removed}, read(t, author), + "an author must be able to see their own posts in every admission state — it is how a client renders "+ + "'pending review' and 'removed' on the author's own profile (PRD §6.2)") + }) +} + +// TestProfileStatsVisibility_PostCountExcludesNonAccepted is the counts predicate +// (PRD §6.2: "counts must not include non-accepted rows"). GetProfileStats' +// post_count is a live COUNT over `posts`, and a count that includes pending or +// removed rows leaks their existence — a profile advertising 4 posts when only 1 +// is visible is a side channel onto content the reader cannot reach. +func TestProfileStatsVisibility_PostCountExcludesNonAccepted(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "ps") + author := "did:plc:vispsauthor" + createTestUser(t, db, "vispsauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "psacc", "accepted", base.Add(3*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "pspen", "pending", base.Add(2*time.Hour)) + removed := seedVisibilityPost(t, db, community, author, "psrem", "removed", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2psacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, removed, posts.AdmissionStatusRemoved, "", "rule-violation") + + repo := NewUserRepository(db) + stats, err := repo.GetProfileStats(ctx, author) + require.NoError(t, err) + + assert.Equalf(t, 1, stats.PostCount, + "a profile's post_count must count accepted posts only; counting the pending and removed rows too (%d seeded, "+ + "1 accepted) advertises the existence of content no reader can reach", 3) +} diff --git a/tests/e2e/read_visibility_contract_test.go b/tests/e2e/read_visibility_contract_test.go new file mode 100644 index 0000000..64a26c2 --- /dev/null +++ b/tests/e2e/read_visibility_contract_test.go @@ -0,0 +1,227 @@ +//go:build e2e + +package e2e + +import ( + "context" + "net/url" + "testing" + + "Coves/tests/testkit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The read-path visibility contract (task 7, PRD §6.2, §9 re-scoped): the +// binding security arc for author-owned posts. It is the compensating control +// for task 6's write-path flip — that flip means any author can index a postv2 +// naming any community, so the ONLY thing standing between an unadmitted post and +// every reader is the admission predicate this task builds. An alternate-endpoint +// hole is a real leak, so the invisibility is asserted on EVERY public display +// endpoint, not just the feed. +// +// # WHAT THIS TIER CAN AND CANNOT OBSERVE +// +// The viewer here is the ANONYMOUS public. §3.4b's standing limitation — nothing +// but the browser OAuth callback mints a sealed session RequireAuth accepts — +// means this tier cannot authenticate a viewer AT ALL, so it proves the one case +// that matters most: the unauthenticated internet must reach accepted content +// only. The author's own privileged view of their pending/removed posts (PRD +// §6.2) needs an authenticated viewer DID and is proven at T1, where the read +// requests carry a ViewerDID directly (internal/db/postgres/post_visibility_test.go, +// TestActorPostsVisibility_AuthorVsNonAuthor). +// +// getTimeline is also absent below for a tier reason, not an oversight: it is the +// one feed behind RequireAuth (routes: authRequired), so an anonymous timeline +// read is a 401 rather than a filtered feed. Its accepted-only predicate is +// proven at T1 (TestTimelineVisibility_AcceptedOnly). +// +// # HOW ADMISSION STATE IS DRIVEN +// +// The same way author_post_contract_test.go drives it, and for the same reason: +// no community in this tier holds PDS credentials, so the production acceptance +// engine cannot run out here. The test writes the acceptance / removal records +// into the community's own repo directly — which is exactly the events a +// credentialed engine would emit — and confirms the resulting admission state +// through getStatus BEFORE probing the display endpoints, so a display leak can +// never be mistaken for the state not having converged yet. + +// feedItemView is the slice of a feed response the visibility probes read. +type feedItemView struct { + Post struct { + URI string `json:"uri"` + } `json:"post"` +} + +// removablePostView reads post.get's union positionally, including the +// #removedPost member (a removed post is served as a tombstone, not omitted). +type removablePostView struct { + URI string `json:"uri"` + NotFound bool `json:"notFound"` + Removed bool `json:"removed"` + Code string `json:"code"` +} + +// communityFeedURIs reads a community feed as the public and returns the URIs it +// served. +func communityFeedURIs(t *testing.T, p *pipeline, communityDID string) []string { + t.Helper() + var out struct { + Feed []feedItemView `json:"feed"` + } + require.NoError(t, p.AppView.Query(context.Background(), "social.coves.communityFeed.getCommunity", + url.Values{"community": {communityDID}, "sort": {"new"}, "limit": {"50"}}, &out)) + uris := make([]string, 0, len(out.Feed)) + for _, item := range out.Feed { + uris = append(uris, item.Post.URI) + } + return uris +} + +// discoverFeedURIsPublic reads the discover feed as the public and returns the +// URIs it served. +func discoverFeedURIsPublic(t *testing.T, p *pipeline) []string { + t.Helper() + var out struct { + Feed []feedItemView `json:"feed"` + } + require.NoError(t, p.AppView.Query(context.Background(), "social.coves.feed.getDiscover", + url.Values{"sort": {"new"}, "limit": {"50"}}, &out)) + uris := make([]string, 0, len(out.Feed)) + for _, item := range out.Feed { + uris = append(uris, item.Post.URI) + } + return uris +} + +// getRemovablePost reads one post.get union member, exposing the #removedPost +// discriminator and code the plain postView helper does not. +func getRemovablePost(t *testing.T, p *pipeline, uri string) removablePostView { + t.Helper() + var out struct { + Posts []removablePostView `json:"posts"` + } + require.NoError(t, p.AppView.Query(context.Background(), "social.coves.community.post.get", + url.Values{"uris": {uri}}, &out)) + require.Lenf(t, out.Posts, 1, "post.get must answer positionally, one member per requested URI") + return out.Posts[0] +} + +// TestReadVisibilityContract is the binding arc: an accepted post is reachable +// through every public display endpoint, a pending one through NONE of them, and +// a removed one renders as #removedPost carrying its code. +// +// It carries NO ingestion-contract marker: markers are for pipeline proofs +// (§3.4a), and postv2's is already owned by TestAuthorPostIngestion. This asserts +// the READ path — what a public client reaches — which is a different contract. +func TestReadVisibilityContract(t *testing.T) { + p := newPipeline(t) + + author := p.IndexedAccount(t, "rvc") + community := indexedCommunity(t, p, "rvc", author.DID) + + // ---- an accepted post: the positive control on every surface ------------ + acceptedRkey := testkit.TID() + acceptedURI := authorPostURI(author.DID, acceptedRkey) + acceptedTitle := "accepted " + testkit.UniqueID(t) + acceptedRecord := author.PutRecord(t, postV2Collection, acceptedRkey, + postV2Record(community.DID, acceptedTitle, "content the community will attest to")) + awaitStatus(t, p, acceptedURI, community.DID, "pending", "the acceptable post to be indexed") + community.PutRecord(t, acceptanceCollection, subjectRkey(acceptedURI), + acceptanceRecord(acceptedURI, acceptedRecord.CID)) + awaitStatus(t, p, acceptedURI, community.DID, "accepted", "the community's acceptance to admit the post") + + // ---- a pending post: never admitted, and it bounds the accepted one ----- + // Written and confirmed pending. Because both posts are in the SAME author + // repo and the pending one is committed after the acceptable one's postv2, + // its indexing has necessarily been through the consumer by the time the + // acceptance above is observed — so its ABSENCE below is meaningful, not just + // "hasn't arrived yet". + pendingRkey := testkit.TID() + pendingURI := authorPostURI(author.DID, pendingRkey) + pendingTitle := "pending " + testkit.UniqueID(t) + author.PutRecord(t, postV2Collection, pendingRkey, + postV2Record(community.DID, pendingTitle, "content no community has agreed to carry")) + awaitStatus(t, p, pendingURI, community.DID, "pending", "the pending post to be indexed and awaiting a decision") + + t.Run("the accepted post is reachable through every public endpoint", func(t *testing.T) { + assert.Containsf(t, communityFeedURIs(t, p, community.DID), acceptedURI, + "an accepted post must appear in its community feed") + assert.Contains(t, discoverFeedURIsPublic(t, p), acceptedURI, + "an accepted post must appear in discover") + + got := getRemovablePost(t, p, acceptedURI) + assert.False(t, got.NotFound, "an accepted post must be served by post.get") + assert.False(t, got.Removed) + + thread, err := p.Thread(context.Background(), acceptedURI, nil) + require.NoError(t, err, "getComments must serve the header of an accepted post") + assert.Equal(t, acceptedURI, thread.Post.URI) + }) + + t.Run("the pending post is invisible on every public endpoint", func(t *testing.T) { + // The security core. Each of these is an alternate path to the same + // content, and a hole in any one of them is a real leak — a client that + // cannot see a pending post in the feed can still permalink it, read it + // through the author feed, or open its comment thread. + assert.NotContainsf(t, communityFeedURIs(t, p, community.DID), pendingURI, + "a PENDING post appeared in the community feed. It has no acceptance; rendering it publishes speech the "+ + "community never agreed to carry (PRD §2)") + assert.NotContainsf(t, discoverFeedURIsPublic(t, p), pendingURI, + "a PENDING post appeared in discover") + + got := getRemovablePost(t, p, pendingURI) + assert.Truef(t, got.NotFound, + "post.get served a PENDING post to an anonymous caller. A pending post must be a notFoundPost to the public, "+ + "or every feed gate is worthless against a direct permalink") + + // getComments hydrates its post header through the post read path, and on + // this branch that path is admission-blind and serves soft-deleted posts + // too (the 2026-07-29 defect). The header of a pending post must not reach + // the public — whether GREEN answers with a not-found error or a hidden + // header, the pending title must never appear. + thread, err := p.Thread(context.Background(), pendingURI, nil) + if err == nil { + assert.NotEqualf(t, pendingTitle, thread.Post.Record["title"], + "getComments exposed a PENDING post's header (title, content, author) to the public through its thread "+ + "endpoint — the alternate-endpoint leak PRD §6.2 names explicitly") + assert.Truef(t, thread.Post.URI == "" || thread.Post.NotFound, + "getComments served a real post header for a pending post: %+v", thread.Post) + } + }) + + // ---- removal: the accepted post is taken down and renders as a tombstone - + t.Run("a removed post renders as #removedPost with its code, everywhere else gone", func(t *testing.T) { + subject := subjectRkey(acceptedURI) + applyWrites(t, community, []map[string]any{ + {"$type": "com.atproto.repo.applyWrites#delete", "collection": acceptanceCollection, "rkey": subject}, + { + "$type": "com.atproto.repo.applyWrites#create", + "collection": removalCollection, + "rkey": subject, + "value": removalRecord(acceptedURI, acceptedRecord.CID, "rule-violation"), + }, + }) + awaitStatus(t, p, acceptedURI, community.DID, "removed", "the atomic removal commit to take the post down") + + // post.get answers the removed member — NOT notFound — carrying the code, + // so a client renders "removed: rule-violation" rather than a blank + // permalink. + p.Await(t, "post.get to serve the removed post as a #removedPost tombstone", func() (bool, error) { + got := getRemovablePost(t, p, acceptedURI) + return got.Removed, nil + }) + got := getRemovablePost(t, p, acceptedURI) + assert.Falsef(t, got.NotFound, + "a removed post must be a #removedPost, not a notFoundPost — the author is owed the reason, not silence") + assert.Equalf(t, "rule-violation", got.Code, + "the removedPost member must carry the removal code; a tombstone without one is an unexplained disappearance") + + // And it is gone from every browsing surface. + assert.NotContainsf(t, communityFeedURIs(t, p, community.DID), acceptedURI, + "a removed post must drop out of the community feed") + assert.NotContains(t, discoverFeedURIsPublic(t, p), acceptedURI, + "a removed post must drop out of discover") + }) +} diff --git a/tests/lexicon_removedpost_test.go b/tests/lexicon_removedpost_test.go new file mode 100644 index 0000000..857a9af --- /dev/null +++ b/tests/lexicon_removedpost_test.go @@ -0,0 +1,163 @@ +package tests + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The lexicon half of task 7's read-path rebuild (PRD §3.4, §6.2). post.get +// grows one union member and postView grows two optional fields, all additive +// so no published consumer breaks: +// +// - social.coves.community.post.get's output union gains #removedPost, the +// mirror of #notFoundPost/#blockedPost — a removed post is served as a +// tombstone carrying its removal code, not silently omitted like a +// never-indexed one, so a client can render "removed by moderators: spam" +// rather than a blank permalink. +// - social.coves.community.post.defs gains the #removedPost object. +// - postView gains optional `status` and `acceptanceUri`, the per-community +// admission context an author's own view renders (PRD §6.2). Optional, so a +// public postView that omits them still validates. +// +// These are asserted against the raw schema JSON rather than the indigo catalog +// because the properties in question are UNOBSERVABLE from record fixtures: +// atproto lexicons are open, so no data sample can prove a union gained a member +// or an object gained an optional field. Reading the schema source is the only +// thing that fails when the additive change is missing — which is the whole +// point of pinning it at T0. + +// postDefsPath and postGetPath are the two schema files this task edits. +const ( + postDefsPath = "../internal/atproto/lexicon/social/coves/community/post/defs.json" + postGetPath = "../internal/atproto/lexicon/social/coves/community/post/get.json" + + removedPostRef = "social.coves.community.post.defs#removedPost" +) + +// admissionStatusValues is the closed vocabulary the admissions table and +// getStatus already speak (migration 034, status.go). postView.status must offer +// the same set so a client switching on it switches on the real state machine. +var admissionStatusValues = []string{"pending", "accepted", "pending_reacceptance", "rejected", "removed"} + +// readLexiconJSON parses a schema file into a generic tree for shape assertions. +func readLexiconJSON(t *testing.T, path string) map[string]interface{} { + t.Helper() + raw, err := os.ReadFile(filepath.Clean(path)) + require.NoErrorf(t, err, "reading lexicon %s", path) + var doc map[string]interface{} + require.NoErrorf(t, json.Unmarshal(raw, &doc), "parsing lexicon %s", path) + return doc +} + +// defProperties reaches defs..properties, failing the test if the def or +// its properties object is missing. +func defProperties(t *testing.T, doc map[string]interface{}, defName string) map[string]interface{} { + t.Helper() + defs, ok := doc["defs"].(map[string]interface{}) + require.True(t, ok, "lexicon has no defs object") + def, ok := defs[defName].(map[string]interface{}) + require.Truef(t, ok, "lexicon has no def %q", defName) + props, ok := def["properties"].(map[string]interface{}) + require.Truef(t, ok, "def %q has no properties object", defName) + return props +} + +func asStrings(t *testing.T, v interface{}, what string) []string { + t.Helper() + arr, ok := v.([]interface{}) + require.Truef(t, ok, "%s is not an array (got %T)", what, v) + out := make([]string, 0, len(arr)) + for _, item := range arr { + s, ok := item.(string) + require.Truef(t, ok, "%s contains a non-string element %v", what, item) + out = append(out, s) + } + return out +} + +// TestPostGetUnionCarriesRemovedPost pins that post.get's output union offers +// #removedPost. A removed post that came back as #notFoundPost would tell the +// author their post vanished; the distinct member is what lets the client +// explain the removal. +func TestPostGetUnionCarriesRemovedPost(t *testing.T) { + doc := readLexiconJSON(t, postGetPath) + + main, ok := doc["defs"].(map[string]interface{})["main"].(map[string]interface{}) + require.True(t, ok, "get.json has no main def") + output := main["output"].(map[string]interface{}) + schema := output["schema"].(map[string]interface{}) + props := schema["properties"].(map[string]interface{}) + postsProp, ok := props["posts"].(map[string]interface{}) + require.True(t, ok, "get.json main output has no posts property") + items, ok := postsProp["items"].(map[string]interface{}) + require.True(t, ok, "posts property has no items") + + refs := asStrings(t, items["refs"], "post.get union refs") + assert.Containsf(t, refs, removedPostRef, + "post.get's output union does not offer %s. A removed post must be served as its own tombstone member carrying "+ + "the removal code, not collapsed into notFoundPost (which is indistinguishable from 'never existed') or "+ + "silently omitted", removedPostRef) +} + +// TestRemovedPostDefShape pins the #removedPost object: it mirrors notFoundPost +// (uri + a const discriminator) and additionally carries the removal `code` a +// client renders to the author. +func TestRemovedPostDefShape(t *testing.T) { + doc := readLexiconJSON(t, postDefsPath) + + defs := doc["defs"].(map[string]interface{}) + def, ok := defs["removedPost"].(map[string]interface{}) + require.True(t, ok, "post/defs.json has no removedPost def — the union member has nothing to resolve to") + + required := asStrings(t, def["required"], "removedPost required") + assert.Contains(t, required, "uri", "removedPost must echo the URI it is about") + assert.Contains(t, required, "removed", "removedPost must carry a boolean discriminator, like notFoundPost/blockedPost") + + props := def["properties"].(map[string]interface{}) + require.Contains(t, props, "code", + "removedPost must carry the removal code — a removal without one is an unexplained disappearance (PRD §3.3)") + + removedDiscriminator, ok := props["removed"].(map[string]interface{}) + require.True(t, ok, "removedPost has no removed property") + assert.Equal(t, true, removedDiscriminator["const"], + "the removed discriminator must be const true, so a client can tell this union member apart structurally") +} + +// TestPostViewCarriesAdmissionContext pins the two optional fields postView +// gains: status and acceptanceUri (PRD §6.2). Both must be OPTIONAL — a public +// postView that omits them still validates — and status must offer the full +// admission vocabulary so an author's client renders the real state. +func TestPostViewCarriesAdmissionContext(t *testing.T) { + doc := readLexiconJSON(t, postDefsPath) + + props := defProperties(t, doc, "postView") + + status, ok := props["status"].(map[string]interface{}) + require.True(t, ok, "postView has no status property — an author's own view cannot render 'pending'/'removed' without it") + assert.Equal(t, "string", status["type"], "postView.status must be a string") + known := asStrings(t, status["knownValues"], "postView.status knownValues") + assert.ElementsMatchf(t, admissionStatusValues, known, + "postView.status must offer the same admission vocabulary the table and getStatus speak (%v), so a client "+ + "switches on the real state machine and not a display translation", admissionStatusValues) + + acceptanceURI, ok := props["acceptanceUri"].(map[string]interface{}) + require.True(t, ok, "postView has no acceptanceUri property — a client cannot follow the acceptance to verify it") + assert.Equal(t, "string", acceptanceURI["type"]) + assert.Equal(t, "at-uri", acceptanceURI["format"], "acceptanceUri must be an at-uri so it resolves to the acceptance record") + + // Both fields are ADDITIVE-OPTIONAL: a postView without them must still be + // valid, or every existing consumer breaks. postView's required set is the + // published seven and must not have grown. + defs := doc["defs"].(map[string]interface{}) + postView := defs["postView"].(map[string]interface{}) + required := asStrings(t, postView["required"], "postView required") + assert.NotContains(t, required, "status", "status must be optional — a public postView omits it") + assert.NotContains(t, required, "acceptanceUri", "acceptanceUri must be optional — only an accepted post has one") + assert.ElementsMatch(t, []string{"uri", "cid", "author", "record", "community", "createdAt", "indexedAt"}, required, + "postView's required set must stay exactly the published seven; the admission context is additive-optional") +} -- 2.51.2 From 4c57e3772524e76bbc121d34675323c98a6af8ec Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 08:16:22 -0700 Subject: [PATCH 2/6] =?UTF-8?q?test(posts):=20RED=20cycle=202=20=E2=80=94?= =?UTF-8?q?=20collection-aware=20fail-closed=20+=20getComments=20+=20count?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrects cycle 1's fail-OPEN predicate (missing admission row => visible), which is a security hole for postv2. - TestVisibility_CollectionAwareFailClosed (pinned hardest): a legacy community.post with no admission row stays visible (accepted-by-construction, until task 8's drain); a postv2 with no admission row is HIDDEN from non-authors on feed/discover/post.get but visible to its author. A missing postv2 admission row is a failed pending seed, not grandfathered content. - TestGetCommentsVisibility: the thread header goes through an admission+ deleted-aware fetch — pending header hidden from non-author, accepted shown, soft-deleted no longer leaks (closes the 2026-07-29 defect). - TestCommunityPostCountVisibility + countAcceptedPostsForCommunity stub: pins accepted-only count semantics (community post_count is a stale stored counter; the counter itself is a consumer-side follow-up — see report). - Blocker reseed: service_blob_test.go's PDS-hydration fixture gets an accepted admission row (it tests hydration, not visibility) so the fail-closed fix doesn't break it. - author_post_contract retarget/delete rework: the anonymous content reads of a now-hidden pending post are replaced — retarget leans on getStatus state; the delete accepts the post first so its removal is an observable served->gone transition. Authorship-from-repo proof moved onto the accepted post in the read-visibility contract. Co-Authored-By: Claude Fable 5 --- internal/core/posts/service_blob_test.go | 13 ++ internal/db/postgres/post_visibility.go | 104 ++++++--- internal/db/postgres/post_visibility_test.go | 220 +++++++++++++++++++ tests/e2e/author_post_contract_test.go | 56 +++-- tests/e2e/read_visibility_contract_test.go | 11 + 5 files changed, 359 insertions(+), 45 deletions(-) diff --git a/internal/core/posts/service_blob_test.go b/internal/core/posts/service_blob_test.go index f5ab42a..72a4950 100644 --- a/internal/core/posts/service_blob_test.go +++ b/internal/core/posts/service_blob_test.go @@ -356,6 +356,19 @@ func TestService_AuthorPDSIsHydratedOntoPostViews(t *testing.T) { `, uri, "bafyblobowner", rkey, f.author.DID, f.community.DID, "a post with media") require.NoError(t, err) + // This subject is a postv2, so the task-7 visibility predicate hides it from + // the anonymous GetViewsByURIs read below unless a community has accepted it — + // a postv2 with no admission row fails CLOSED (the consumer always seeds a + // pending row, so a missing one means a failed seed). This test is about PDS + // HYDRATION on a VISIBLE row, not visibility, so it needs the row to be + // visible: seed the accepted admission the acceptance engine would have + // written. + _, err = f.db.ExecContext(ctx, ` + INSERT INTO community_post_admissions (community_did, post_uri, status, accepted_cid, evaluated_cid, last_community_rev, last_community_op_rank, created_at, updated_at) + VALUES ($1, $2, 'accepted', $3, $3, '3lqqqqqqqqqq2', 1, NOW(), NOW()) + `, f.community.DID, uri, "bafyblobowner") + require.NoError(t, err) + views, err := postgres.NewPostRepository(f.db).GetViewsByURIs(ctx, []string{uri}) require.NoError(t, err) require.Contains(t, views, uri) diff --git a/internal/db/postgres/post_visibility.go b/internal/db/postgres/post_visibility.go index 6168b33..51a2939 100644 --- a/internal/db/postgres/post_visibility.go +++ b/internal/db/postgres/post_visibility.go @@ -1,37 +1,89 @@ package postgres +import ( + "context" + "database/sql" + "fmt" +) + // The centralized read-path visibility predicate (task 7, PRD §6.2). // -// STUB — signature only. This is the single admission-aware join every posts -// display query must go through so that no read path can forget the gate (the -// piecemeal-predicate failure PRD §6.2 calls out). GREEN fills in the body and -// wires it into GetViewsByURIs, the three feed queries, GetByAuthor and the -// profile count; the visibility suites in post_visibility_test.go are red until -// it does. +// This is the single admission-aware gate every posts display query goes +// through so that no read path can forget it (the piecemeal-predicate failure +// PRD §6.2 calls out). It is wired into GetViewsByURIs, the three feed queries +// and GetByAuthor; the profile/community counts apply the same accepted rule +// inline (a COUNT subquery has no row to hydrate). +// +// # The join key is (a.community_did = p.community_did AND a.post_uri = p.uri) // -// THE JOIN KEY IS (a.community_did = p.community_did AND a.post_uri = p.uri). // Both halves are load-bearing. The post_uri half selects the subject; the // community_did half is what makes a post visible iff ITS OWN community accepted // it, which is the fork-case security property TestDiscoverVisibility_ForkJoinKey // pins — a join on post_uri alone would let one community's acceptance publish -// another community's pending post. -// -// THE STATUS RULE depends on the viewer: -// - a non-author (viewerDID == "" or != the post's author): status = 'accepted' -// only. -// - the author of the post (viewerDID == posts.author_did): accepted OR the -// author's own non-accepted rows, so a client can render 'pending' / 'removed' -// on the author's own profile. -// -// visiblePostsJoin returns the SQL fragment to splice after the posts `p` -// reference (a JOIN plus its WHERE contribution) and the arguments it binds, -// starting at paramOffset. Returning the empty fragment — the stub's behavior — -// applies NO gate, which is the pre-task-7 state every visibility suite fails -// against. -func visiblePostsJoin(viewerDID string, paramOffset int) (sqlFragment string, args []interface{}) { - return "", nil +// another community's pending post. Because p.community_did and p.uri are fixed +// per posts row and community_post_admissions is unique on (community_did, +// post_uri), at most one admission row can match, so the LEFT JOIN never +// duplicates a post. +// +// # The status rule +// +// The gate turns on the admission row the LEFT JOIN produced for the post's OWN +// community: +// +// - a.status = 'accepted' → visible to everyone. +// - a.status IS NULL → visible. No admission row exists for this +// (community, post): a legacy community-repo post, a bridged post, or any +// other collection that never goes through the admission engine, plus the +// narrow window before the consumer opens a fresh postv2's pending row +// (authorpost.go writes the post and its pending admission in separate +// transactions). These carry no decision to gate on, so they stay visible +// exactly as they were before task 7 — the read path is not the place to +// retro-hide content that predates admissions. +// - a.status IN (pending, pending_reacceptance, removed, rejected) AND the +// viewer is the author → visible. An author sees their own posts in every +// admission state so a client can render "pending review" / "removed" on the +// author's own profile (PRD §6.2). Any OTHER viewer — including the anonymous +// public, whose DID is "" — sees accepted content only. This is the security +// core: every non-accepted post that HAS a decision is invisible to +// non-authors on every read path. +// +// visiblePostsJoin returns the JOIN clause to splice into the FROM/JOIN section +// after `FROM posts p`, and the boolean WHERE fragment to AND into the query's +// WHERE clause. The caller owns the arguments: it must bind $viewerParam to the +// viewer's DID (or "" for an anonymous read), reusing a parameter it has already +// bound where the viewer DID is already in the argument list (the timeline +// reuses $1). +func visiblePostsJoin(viewerParam int) (joinSQL, whereSQL string) { + joinSQL = ` + LEFT JOIN community_post_admissions a + ON a.community_did = p.community_did AND a.post_uri = p.uri` + + whereSQL = fmt.Sprintf(`( + a.status = 'accepted' + OR a.status IS NULL + OR (a.status IN ('pending', 'pending_reacceptance', 'removed', 'rejected') AND p.author_did = $%d) + )`, viewerParam) + + return joinSQL, whereSQL +} + +// countAcceptedPostsForCommunity is the accepted-only source of truth for a +// community's post_count (task 7, PRD §6.2). +// +// STUB — returns 0 until GREEN implements it. It counts the posts a community has +// ACCEPTED: a join of `posts` to community_post_admissions on the subject key +// with status = 'accepted'. It exists because community.post_count is a STORED +// column whose only incrementer is the old community-repo write path +// (community_repo_memberships.go) — nothing advances it on an acceptance, so it +// is stale under author-owned posts. Whether GREEN recomputes the count live or +// reconciles the stored column from the admission consumer, THIS is the value it +// must converge on. The read paths already exclude non-accepted rows, so this is +// a counting concern, not a content leak — see the cycle-2 report for the +// recommendation to sequence the counter itself as a consumer-side follow-up. +func countAcceptedPostsForCommunity(ctx context.Context, db *sql.DB, communityDID string) (int, error) { + return 0, nil } -// Referenced so the stub is not flagged as dead before GREEN wires it into the -// read queries. Delete this line once visiblePostsJoin has a real caller. -var _ = visiblePostsJoin +// Referenced so the count stub is not flagged as dead before GREEN wires it into +// the community counter. Delete this line once it has a real caller. +var _ = countAcceptedPostsForCommunity diff --git a/internal/db/postgres/post_visibility_test.go b/internal/db/postgres/post_visibility_test.go index 6e85edb..98f03ae 100644 --- a/internal/db/postgres/post_visibility_test.go +++ b/internal/db/postgres/post_visibility_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "Coves/internal/core/comments" "Coves/internal/core/communityFeeds" "Coves/internal/core/discover" "Coves/internal/core/posts" @@ -391,3 +392,222 @@ func TestProfileStatsVisibility_PostCountExcludesNonAccepted(t *testing.T) { "a profile's post_count must count accepted posts only; counting the pending and removed rows too (%d seeded, "+ "1 accepted) advertises the existence of content no reader can reach", 3) } + +// TestVisibility_CollectionAwareFailClosed is the corrected core of task 7, and +// the single most important assertion in this suite. Cycle 1's predicate went +// FAIL-OPEN: a post with no admission row was visible to everyone. That is +// correct for a LEGACY community.post — it was signed into the community's own +// repo under the old model, is accepted by construction, and must stay visible +// until task 8 drains it — but it is a security HOLE for a postv2, because the +// task-5 consumer ALWAYS seeds a pending admission the moment it indexes a +// postv2. A postv2 with NO admission row therefore does not mean "pre-admission +// content to grandfather in"; it means the seed has not happened (or failed), +// and the right answer is to FAIL CLOSED. +// +// So the no-admission rule is COLLECTION-AWARE, discriminated by the collection +// segment of the post URI (CollectionOfPostURI): legacy → visible, postv2 → +// hidden from non-authors, visible to its author. Both posts below carry no +// admission row at all; only their collection differs. +func TestVisibility_CollectionAwareFailClosed(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "fc2") + author := "did:plc:visfc2author" + createTestUser(t, db, "visfc2author.test", author) + stranger := "did:plc:visfc2stranger" + createTestUser(t, db, "visfc2stranger.test", stranger) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + + // A legacy community-repo post, no admission row: accepted-by-construction, + // must stay visible to the public until task 8's drain. + legacy := seedFilterablePost(t, db, community, author, "fc2leg", base.Add(2*time.Hour)) + // A postv2, no admission row: the consumer would have seeded pending, so a + // missing row is a failed seed — fail closed for non-authors. + postv2 := seedVisibilityPost(t, db, community, author, "fc2pv2", "postv2 with no admission", base.Add(1*time.Hour)) + + feedRepo := NewCommunityFeedRepository(db, "test-secret") + postRepo := NewPostRepository(db) + discoverRepo := NewDiscoverRepository(db, "test-secret") + + communityFeed := func(t *testing.T, viewer string) []string { + t.Helper() + feed, _, err := feedRepo.GetCommunityFeed(ctx, communityFeeds.GetCommunityFeedRequest{ + Community: community, ViewerDID: viewer, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + return feedURIs(feed) + } + + t.Run("the legacy post with no admission row stays visible to the public", func(t *testing.T) { + assert.Containsf(t, communityFeed(t, publicViewer), legacy, + "a legacy community.post with no admission row must stay visible — it was accepted by construction under the "+ + "old model and task 7 must not retro-hide content that predates admissions (visible until task 8's drain)") + + views, err := postRepo.GetViewsByURIs(ctx, []string{legacy}) + require.NoError(t, err) + assert.Contains(t, views, legacy, "post.get must still serve a legacy post with no admission row to the public") + }) + + t.Run("the postv2 with no admission row is HIDDEN from non-authors everywhere", func(t *testing.T) { + // The corrected security core. A missing admission row on a postv2 is a + // failed pending seed, not grandfathered content — so it must fail closed + // on every display surface for anyone who is not its author. + assert.NotContainsf(t, communityFeed(t, publicViewer), postv2, + "a postv2 with no admission row leaked to the public in the community feed. The consumer always seeds a "+ + "pending row on index, so no-row means the seed failed — the read path must FAIL CLOSED for postv2, "+ + "not fail open as it does for legacy posts (this is the hole this task closes)") + assert.NotContainsf(t, communityFeed(t, stranger), postv2, + "a postv2 with no admission row leaked to a non-author (an authenticated stranger) in the community feed") + + disc, _, err := discoverRepo.GetDiscover(ctx, discover.GetDiscoverRequest{ + ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + assert.NotContains(t, discoverFeedURIs(disc), postv2, + "a postv2 with no admission row leaked into discover") + + views, err := postRepo.GetViewsByURIs(ctx, []string{postv2}) + require.NoError(t, err) + assert.NotContainsf(t, views, postv2, + "post.get served a postv2 with no admission row to the public — permalink is the alternate path the feed "+ + "gate is worthless without") + }) + + t.Run("the postv2 with no admission row is visible to its own author", func(t *testing.T) { + // A failed/absent seed must not cost the AUTHOR their own post. On the + // surfaces that thread a viewer DID (the feed does), the author sees their + // own postv2 even with no admission row, exactly as they see their own + // pending one. + assert.Containsf(t, communityFeed(t, author), postv2, + "the author of a postv2 with no admission row cannot see their own post in the community feed; a missing "+ + "seed must fail closed for OTHERS, never for the author") + + // NOTE — post.get's author path is a known follow-up, not covered here. + // GetViewsByURIs takes no viewer DID (posts.Repository's 2-arg signature), + // so post.get currently hides a non-accepted postv2 from its author too. + // Threading a viewer would break the three in-suite Repository fakes, so it + // is deferred: the author reaches their own pending/no-admission posts + // through actor.getPosts (TestActorPostsVisibility_AuthorVsNonAuthor) and + // getStatus, which is sufficient. Flagged in the cycle-2 report. + }) +} + +// TestGetCommentsVisibility_HeaderIsAdmissionAndDeleteAware closes the read-path +// hole getComments has carried since 2026-07-29. GetComments hydrates its thread +// header through postRepo.GetByURI, which has NO admission gate and NO +// `deleted_at IS NULL` filter — so the comment thread endpoint serves the full +// header (title, content, author) of a post the feeds correctly hide: a pending +// postv2, and a soft-deleted post. The header must go through the same +// admission+deleted-aware fetch the feeds use. +// +// Driven through the comment SERVICE rather than a bare repo call, because the +// defect is in which fetch GetComments chooses — a repo-only test could not see +// it pick the leaky one. +func TestGetCommentsVisibility_HeaderIsAdmissionAndDeleteAware(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "gc") + author := "did:plc:visgcauthor" + createTestUser(t, db, "visgcauthor.test", author) + stranger := "did:plc:visgcstranger" + createTestUser(t, db, "visgcstranger.test", stranger) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "gcacc", "accepted header", base.Add(3*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "gcpen", "pending header", base.Add(2*time.Hour)) + deleted := seedVisibilityPost(t, db, community, author, "gcdel", "deleted header", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2gcacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, deleted, posts.AdmissionStatusAccepted, "bafypostv2gcdel", "") + + // Soft-delete the third post the way the consumer does. + _, err := db.ExecContext(ctx, `UPDATE posts SET deleted_at = NOW() WHERE uri = $1`, deleted) + require.NoError(t, err) + + service := comments.NewCommentServiceWithPDSFactory( + NewCommentRepository(db), + NewUserRepository(db), + NewPostRepository(db), + NewCommunityRepository(db), + nil, nil, + ) + + header := func(t *testing.T, postURI, viewerDID string) (*comments.GetCommentsResponse, error) { + t.Helper() + var viewer *string + if viewerDID != "" { + viewer = &viewerDID + } + return service.GetComments(ctx, &comments.GetCommentsRequest{PostURI: postURI, ViewerDID: viewer}) + } + + t.Run("an accepted post serves its header", func(t *testing.T) { + resp, err := header(t, accepted, stranger) + require.NoError(t, err, "getComments must serve the header of an accepted post") + require.NotNil(t, resp.Post) + postView, ok := resp.Post.(*posts.PostView) + require.Truef(t, ok, "getComments post header is %T, not *posts.PostView", resp.Post) + assert.Equal(t, accepted, postView.URI) + }) + + t.Run("a pending post's header is hidden from a non-author", func(t *testing.T) { + _, err := header(t, pending, stranger) + require.Errorf(t, err, "getComments served a non-author the header of a PENDING post — the alternate-endpoint "+ + "leak PRD §6.2 names: a post hidden from the feed is fully readable through its comment thread") + assert.ErrorIs(t, err, comments.ErrRootNotFound, + "a pending post must be root-not-found to a non-author's getComments, the same answer post.get gives") + }) + + t.Run("a soft-deleted post no longer leaks (closes the 2026-07-29 defect)", func(t *testing.T) { + _, err := header(t, deleted, stranger) + require.Errorf(t, err, "getComments served the full header of a SOFT-DELETED post. GetByURI has no deleted_at "+ + "filter, so the withdrawn post's title/content/author are still returned through the thread endpoint — the "+ + "defect filed 2026-07-29") + assert.ErrorIs(t, err, comments.ErrRootNotFound) + }) +} + +// TestCommunityPostCountVisibility_AcceptedOnly pins that a community's +// post_count reflects accepted posts only (PRD §6.2: counts must not include +// non-accepted rows). +// +// UNLIKE the user post_count, which is a live COUNT this task can gate directly, +// community.post_count is a STORED column with a write-time incrementer +// (community_repo_memberships.go) left over from the old community-repo write +// path. Under author-owned posts nothing increments it on acceptance, so it is +// already stale — and the honest fix is consumer-side (increment on the accept +// transition, decrement on remove/unaccept), NOT a read predicate. +// +// This pins the accepted-only SEMANTICS against countAcceptedPostsForCommunity — +// the source of truth GREEN would drive the counter from, whether it recomputes +// live or reconciles the stored column from the admission consumer. See the +// cycle-2 report for the sequencing recommendation (this is a consumer-side +// follow-up, not part of the read-path predicate). +func TestCommunityPostCountVisibility_AcceptedOnly(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "cc") + author := "did:plc:visccauthor" + createTestUser(t, db, "visccauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + accepted := seedVisibilityPost(t, db, community, author, "ccacc", "accepted", base.Add(3*time.Hour)) + pending := seedVisibilityPost(t, db, community, author, "ccpen", "pending", base.Add(2*time.Hour)) + removed := seedVisibilityPost(t, db, community, author, "ccrem", "removed", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2ccacc", "") + seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, removed, posts.AdmissionStatusRemoved, "", "rule-violation") + + count, err := countAcceptedPostsForCommunity(ctx, db, community) + require.NoError(t, err) + assert.Equalf(t, 1, count, + "a community's accepted-post count must be 1 (3 seeded: accepted, pending, removed); a count that includes "+ + "non-accepted rows advertises content no reader can reach") +} diff --git a/tests/e2e/author_post_contract_test.go b/tests/e2e/author_post_contract_test.go index d840a1c..af1963f 100644 --- a/tests/e2e/author_post_contract_test.go +++ b/tests/e2e/author_post_contract_test.go @@ -242,22 +242,20 @@ func TestAuthorPostIngestion(t *testing.T) { assert.Empty(t, view.AcceptanceURI, "a pending post has no acceptance record to point at") assert.Empty(t, view.DecisionCode, "a pending post has been refused by nobody") - // The post itself is served, attributed to the repo it arrived in. post.get - // is status-agnostic today — task 7 owes the centralized visibility - // predicate that makes a pending post invisible to non-authors (§6.2) — so - // what is asserted here is what IS true: the record was indexed, and its - // author is the DID that signed the commit rather than a field somebody - // could have written. + // The post is INVISIBLE to the anonymous public through post.get. Task 7's + // centralized visibility predicate (§6.2) hides any non-accepted postv2 from a + // non-author, and absence from the view set becomes a notFoundPost on the + // wire. This is the compensating control for the write-path flip: a post any + // author can index naming any community must not render as that community's + // content until the community admits it. (The record WAS indexed — getStatus + // above reports it pending — and its author's own privileged view of it is a + // T1 concern, since this tier can only read as the anonymous public.) served, err := p.Post(context.Background(), uri) require.NoError(t, err) - require.Falsef(t, served.NotFound, "the indexed post must be served by post.get: %+v", served) - assert.Equalf(t, author.DID, served.Author.DID, - "authorship must come from the repo the commit arrived in; the postv2 record carries no author field at all, so a different DID here means one was invented") - assert.Equal(t, community.DID, served.Community.DID) - assert.Equal(t, record.CID, served.CID, "the indexed CID must be the commit's") - assert.Equal(t, title, served.Record["title"]) - assert.Nilf(t, served.Record["author"], - "the record must not carry an author field: it is the field whose removal makes authorship unforgeable (§3.1)") + assert.Truef(t, served.NotFound, + "a PENDING post must be a notFoundPost to the anonymous public through post.get, or every feed gate is worthless against a direct permalink: %+v", served) + assert.NotEqualf(t, record.CID, served.CID, + "a notFoundPost must leak nothing about the unadmitted post it stands in for — not even the committed CID") // ---- retarget: the whole event is invalid ------------------------------ // §3.1 is explicit — a consumer must DISCARD an update that changes @@ -295,13 +293,33 @@ func TestAuthorPostIngestion(t *testing.T) { assert.Equal(t, "pending", original.Status, "the original community's decision must be untouched by an invalid update") - unchanged, err := p.Post(context.Background(), uri) - require.NoError(t, err) - assert.Equalf(t, title, unchanged.Record["title"], - "the CONTENT of a discarded event must be discarded with it: applying the new title while refusing the new community would leave the community holding a CID it never judged") - assert.NotEqual(t, retargeted, unchanged.Record["title"]) + // The CONTENT half of the discard — that the original post's row still holds + // the pre-retarget title and CID rather than the retargeted ones — is no + // longer observable here: task 7 hides a pending postv2 from the anonymous + // public, so post.get answers notFoundPost and there is no record to read the + // title out of. It is asserted at T1 against the consumer's stored row + // (internal/atproto/jetstream/postv2_consumer_test.go). What this tier proves + // is the STATE truth via getStatus: the retarget opened no admission + // elsewhere and left the original community's pending decision untouched. // ---- delete ------------------------------------------------------------- + // A delete is only observable as a TRANSITION, and a pending post is already + // invisible to the public — so to prove the delete does anything, the post is + // first ACCEPTED (making it publicly served), then deleted. The acceptance's + // pinned CID is the original record's; the retarget above was discarded, so + // the row still holds it. + acceptRkey := subjectRkey(uri) + community.PutRecord(t, acceptanceCollection, acceptRkey, acceptanceRecord(uri, record.CID)) + awaitStatus(t, p, uri, community.DID, "accepted", "the post to be accepted so its deletion is an observable transition") + + p.Await(t, "the accepted post to be served before it is deleted", func() (bool, error) { + v, err := p.Post(context.Background(), uri) + if err != nil { + return false, err + } + return !v.NotFound, nil + }) + author.DeleteExistingRecord(t, postV2Collection, rkey) gone := func() (bool, error) { diff --git a/tests/e2e/read_visibility_contract_test.go b/tests/e2e/read_visibility_contract_test.go index 64a26c2..00e15cd 100644 --- a/tests/e2e/read_visibility_contract_test.go +++ b/tests/e2e/read_visibility_contract_test.go @@ -155,6 +155,17 @@ func TestReadVisibilityContract(t *testing.T) { assert.False(t, got.NotFound, "an accepted post must be served by post.get") assert.False(t, got.Removed) + // Authorship comes from the repo the commit arrived in, not a self-asserted + // field — the postv2 record has no author field at all (§3.1). This proof + // moved here from TestAuthorPostIngestion, which can no longer make it on a + // pending post now that the predicate hides one from the anonymous public. + full, err := p.Post(context.Background(), acceptedURI) + require.NoError(t, err) + assert.Equalf(t, author.DID, full.Author.DID, + "the accepted post's author must be the repo DID that signed the commit; a different DID here means an author field was invented") + assert.Nilf(t, full.Record["author"], + "a postv2 record must carry no author field — its absence is what makes authorship unforgeable (§3.1)") + thread, err := p.Thread(context.Background(), acceptedURI, nil) require.NoError(t, err, "getComments must serve the header of an accepted post") assert.Equal(t, acceptedURI, thread.Post.URI) -- 2.51.2 From 5e3b0d0d859ce25f0fa7c1fadddbc1c02031176c Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 08:17:51 -0700 Subject: [PATCH 3/6] =?UTF-8?q?feat(posts):=20GREEN=20cycle=201=20?= =?UTF-8?q?=E2=80=94=20visibility=20predicate=20across=20the=205=20display?= =?UTF-8?q?=20sites=20(belated=20gate=20commit)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Committed late — the every-gate rule slipped: this is GREEN's cycle-1 production (visiblePostsJoin threaded into feed/discover/timeline/ post.get/actor queries, INNER→LEFT users join + scanPostView null handling, #removedPost lexicon + postView status context, user post_count predicate). post_visibility.go's cycle-1 fill was swept into 4c57e37 by RED's add-all; the rest lands here. NOTE: this cycle went FAIL-OPEN on missing admission rows — RED's cycle-2 tests (4c57e37) correctly pin the collection-aware fail-closed correction GREEN implements next. Includes PRD rev 2.8 (read-path inventory). Co-Authored-By: Claude Fable 5 --- docs/PRD_AUTHOR_OWNED_POSTS.md | 11 +++ .../social/coves/community/post/defs.json | 40 +++++++++++ .../social/coves/community/post/get.json | 1 + internal/core/posts/post.go | 50 ++++++++++--- internal/core/posts/service.go | 67 ++++++++++++++++-- internal/db/postgres/discover_repo.go | 29 +++++--- internal/db/postgres/feed_repo.go | 27 ++++--- internal/db/postgres/post_repo.go | 70 +++++++++++++++---- internal/db/postgres/timeline_repo.go | 13 +++- internal/db/postgres/user_repo.go | 11 ++- 10 files changed, 269 insertions(+), 50 deletions(-) diff --git a/docs/PRD_AUTHOR_OWNED_POSTS.md b/docs/PRD_AUTHOR_OWNED_POSTS.md index 0a3dcbf..a230fa0 100644 --- a/docs/PRD_AUTHOR_OWNED_POSTS.md +++ b/docs/PRD_AUTHOR_OWNED_POSTS.md @@ -39,6 +39,17 @@ explicitly deferred to Beta. Rev 2.6 (2026-08-08): task-3 second-opinion — fingerprint normalized to resolved-DID scope, release decoupled from request context, admission wiring fail-loud, ActorClass fail-closed. +Rev 2.8 (2026-08-08): task-7 read-path inventory — the visibility predicate +is a shared query-builder JOIN (not a SQL view: feeds carry a computed +hot_rank column + a cursor subquery reading posts directly, and author-self- +view is viewer-DID-parameterized). Per-row join key (a.community_did = +p.community_did AND a.post_uri = p.uri) so cross-community feeds resolve each +post against ITS community's decision (fork case). INNER→LEFT users join +(unknown-author visibility) pairs mandatorily with scanPostView null-handling +(handle COALESCE to author_did, PDSURL, blobOwnerOf). getComments is a +separate path (raw GetByURI, no view, actively serves soft-deleted — own +cycle). Community post_count has NO incrementer — consumer/admission-driven, +not a read predicate. Post text search does not exist (negative guard only). Rev 2.7 (2026-08-08): task-5 plan review — post.getStatus pulled forward into task 5 as the T2 observation surface (unauthenticated; mild disclosure of rejected-post status accepted, owner-flagged); hosted-community detection = diff --git a/internal/atproto/lexicon/social/coves/community/post/defs.json b/internal/atproto/lexicon/social/coves/community/post/defs.json index 5d21814..0a1ea49 100644 --- a/internal/atproto/lexicon/social/coves/community/post/defs.json +++ b/internal/atproto/lexicon/social/coves/community/post/defs.json @@ -72,6 +72,23 @@ "viewer": { "type": "ref", "ref": "#viewerState" + }, + "status": { + "type": "string", + "knownValues": [ + "pending", + "accepted", + "pending_reacceptance", + "rejected", + "removed" + ], + "description": "This post's per-community admission status. Present on an accepted post, and on an author's own view of their non-accepted posts; omitted from a public postView of a legacy/bridged row. Optional.", + "maxLength": 64 + }, + "acceptanceUri": { + "type": "string", + "format": "at-uri", + "description": "AT-URI of the community acceptance record that admitted this post. Present only while an acceptance stands. Optional." } } }, @@ -149,6 +166,29 @@ } } }, + "removedPost": { + "type": "object", + "description": "Post was removed by its own community's moderators. Served as a tombstone carrying the removal code — distinct from notFoundPost (which is indistinguishable from 'never existed') so a client can explain the takedown to the author.", + "required": [ + "uri", + "removed" + ], + "properties": { + "uri": { + "type": "string", + "format": "at-uri" + }, + "removed": { + "type": "boolean", + "const": true + }, + "code": { + "type": "string", + "description": "The community's removal decision code (e.g. rule-violation, spam, off-topic).", + "maxLength": 64 + } + } + }, "blockedPost": { "type": "object", "description": "Post is blocked due to viewer blocking author/community, or community moderation", diff --git a/internal/atproto/lexicon/social/coves/community/post/get.json b/internal/atproto/lexicon/social/coves/community/post/get.json index 47ed913..1b0b44d 100644 --- a/internal/atproto/lexicon/social/coves/community/post/get.json +++ b/internal/atproto/lexicon/social/coves/community/post/get.json @@ -35,6 +35,7 @@ "refs": [ "social.coves.community.post.defs#postView", "social.coves.community.post.defs#notFoundPost", + "social.coves.community.post.defs#removedPost", "social.coves.community.post.defs#blockedPost" ] } diff --git a/internal/core/posts/post.go b/internal/core/posts/post.go index 352f08a..8025d4c 100644 --- a/internal/core/posts/post.go +++ b/internal/core/posts/post.go @@ -169,14 +169,29 @@ type BlockedPost struct { Author *BlockedAuthor `json:"author,omitempty"` } +// RemovedPost is a union member of the social.coves.community.post.get output, +// emitted when a found post has been REMOVED by its own community. It mirrors +// notFoundPost (uri + a const discriminator) and additionally carries the removal +// `code`, so a client can render "removed by moderators: rule-violation" rather +// than the blank permalink a notFoundPost would produce. A removed post is a +// tombstone the author is owed the reason for, not a post that never existed. +// Matches social.coves.community.post.defs#removedPost. +type RemovedPost struct { + URI string `json:"uri"` + Removed bool `json:"removed"` // Always true (const per lexicon); discriminates the union on the wire + Code string `json:"code,omitempty"` +} + // PostResult is one ordered element of a GetPosts response. Exactly one of Post, -// Blocked, or NotFound is set: Post when the post was found and visible to the viewer, -// Blocked when the viewer has blocked the author, NotFound when the URI could not be -// resolved. Construct results via the result helpers so the const discriminators -// (notFound/blocked == true) cannot be left unset. +// Blocked, Removed, or NotFound is set: Post when the post was found and visible +// to the viewer, Blocked when the viewer has blocked the author, Removed when the +// post's own community removed it, NotFound when the URI could not be resolved. +// Construct results via the result helpers so the const discriminators +// (notFound/blocked/removed == true) cannot be left unset. type PostResult struct { Post *PostView Blocked *BlockedPost + Removed *RemovedPost NotFound *NotFoundPost } @@ -190,6 +205,12 @@ func notFoundResult(uri string) *PostResult { return &PostResult{NotFound: &NotFoundPost{URI: uri, NotFound: true}} } +// removedResult builds a removedPost union member with its const discriminator set, +// carrying the community's removal code so the client can explain the takedown. +func removedResult(uri, code string) *PostResult { + return &PostResult{Removed: &RemovedPost{URI: uri, Removed: true, Code: code}} +} + // blockedByAuthorResult builds a blockedPost union member (blockedBy "author") with its // const discriminator set. func blockedByAuthorResult(uri, authorDID string) *PostResult { @@ -227,6 +248,10 @@ func (r *PostResult) Member() (interface{}, bool) { member = r.Blocked count++ } + if r.Removed != nil { + member = r.Removed + count++ + } if r.NotFound != nil { member = r.NotFound count++ @@ -271,10 +296,19 @@ type PostView struct { RKey string `json:"rkey"` CID string `json:"cid"` URI string `json:"uri"` - UpvoteCount int `json:"-"` - DownvoteCount int `json:"-"` - Score int `json:"-"` - CommentCount int `json:"-"` + + // Status and AcceptanceURI are the per-community admission context (PRD §6.2), + // populated from the visibility join. Both are additive-optional: a public + // postView carries status "accepted" (and the acceptance URI), while an + // author's own non-accepted post carries "pending"/"removed"/etc.; a + // legacy/bridged row that holds no admission omits them entirely. + Status string `json:"status,omitempty"` + AcceptanceURI string `json:"acceptanceUri,omitempty"` + + UpvoteCount int `json:"-"` + DownvoteCount int `json:"-"` + Score int `json:"-"` + CommentCount int `json:"-"` } // AuthorView represents author information in post views diff --git a/internal/core/posts/service.go b/internal/core/posts/service.go index a586653..62f6f9d 100644 --- a/internal/core/posts/service.go +++ b/internal/core/posts/service.go @@ -1155,13 +1155,24 @@ func (s *postService) GetPosts(ctx context.Context, req GetPostsRequest) ([]*Pos return nil, fmt.Errorf("failed to fetch post views: %w", err) } - // 3. Assemble results in request order; valid-but-absent URIs become notFoundPost + // 3. Assemble results in request order. A visible view is a postView; an + // absent URI is a notFoundPost — UNLESS its own community removed it, in + // which case it becomes a #removedPost tombstone carrying the removal code + // (PRD §3.4/§6.2). The visibility predicate hides a removed post from + // GetViewsByURIs exactly as it hides a pending one, so the removal is + // recovered here from the admission row rather than from the (absent) view. + removed := s.removedMarkers(ctx, req.URIs, views) results := make([]*PostResult, len(req.URIs)) for i, uri := range req.URIs { - if view := views[uri]; view != nil { - results[i] = foundResult(view) - } else { - results[i] = notFoundResult(uri) + switch { + case views[uri] != nil: + results[i] = foundResult(views[uri]) + default: + if code, ok := removed[uri]; ok { + results[i] = removedResult(uri, code) + } else { + results[i] = notFoundResult(uri) + } } } @@ -1177,6 +1188,52 @@ func (s *postService) GetPosts(ctx context.Context, req GetPostsRequest) ([]*Pos return results, nil } +// removedMarkers returns, for the requested URIs absent from the visible view +// set, the removal code of any whose OWN community removed it — so post.get can +// serve a #removedPost tombstone (PRD §3.4) instead of collapsing a moderator +// removal into an indistinguishable notFoundPost. The presence of a URI in the +// returned map is the removed signal; the value is the code (possibly empty). +// +// It is a no-op when the admissions store is not wired (minimal setups and unit +// tests), leaving every absent URI a plain notFound — the pre-task-7 behavior. +func (s *postService) removedMarkers(ctx context.Context, uris []string, views map[string]*PostView) map[string]string { + markers := make(map[string]string) + if s.admissions == nil { + return markers + } + + seen := make(map[string]struct{}, len(uris)) + for _, uri := range uris { + if views[uri] != nil { + continue // visible — not a candidate for a tombstone + } + if _, done := seen[uri]; done { + continue + } + seen[uri] = struct{}{} + + // A removal is an admission-state change, not a soft delete, so the post + // row still stands and its own community — the key the admission is + // scoped by — comes straight off it. A URI with no row is genuinely + // not-indexed and stays a notFound. + post, err := s.repo.GetByURI(ctx, uri) + if err != nil { + continue + } + admission, err := s.admissions.Get(ctx, post.CommunityDID, uri) + if err != nil || admission == nil || admission.Status != AdmissionStatusRemoved { + continue + } + + code := "" + if admission.DecisionCode != nil { + code = *admission.DecisionCode + } + markers[uri] = code + } + return markers +} + // applyViewerBlocks rewrites found posts whose author the viewer has blocked into // blockedPost results (blockedBy "author"). It batches the block lookup over the unique // author DIDs in the result set. On lookup failure it returns an error rather than diff --git a/internal/db/postgres/discover_repo.go b/internal/db/postgres/discover_repo.go index bb47754..072d40e 100644 --- a/internal/db/postgres/discover_repo.go +++ b/internal/db/postgres/discover_repo.go @@ -44,32 +44,41 @@ func (r *postgresDiscoverRepo) GetDiscover(ctx context.Context, req discover.Get selectClause = feedPostSelectClause("NULL::numeric") } - // Build optional viewer block filter (only when authenticated viewer is present) + // The admission visibility gate always runs. Discover spans every community, + // so it cannot lean on a community filter — the gate keys the admission row on + // (a.community_did = p.community_did AND a.post_uri = p.uri) so a post is + // visible iff ITS OWN community accepted it, never a community that forked it. + // Its viewer parameter ($visibilityParam) is empty for the anonymous public, + // which sees accepted content only. + visibilityParam := 2 + len(cursorValues) + visJoin, visWhere := visiblePostsJoin(visibilityParam) + + // The viewer block filter reuses that same viewer parameter; only meaningful + // for an authenticated viewer, absent for the public. var viewerFilter string - var viewerArgs []interface{} if req.ViewerDID != "" { - viewerParamIdx := 2 + len(cursorValues) - viewerFilter = fmt.Sprintf("AND NOT EXISTS (SELECT 1 FROM user_blocks WHERE blocker_did = $%d AND blocked_did = p.author_did)", viewerParamIdx) - viewerArgs = append(viewerArgs, req.ViewerDID) + viewerFilter = fmt.Sprintf("AND NOT EXISTS (SELECT 1 FROM user_blocks WHERE blocker_did = $%d AND blocked_did = p.author_did)", visibilityParam) } // No subscription filter - show ALL posts from ALL communities query := fmt.Sprintf(` %s - INNER JOIN users u ON p.author_did = u.did - INNER JOIN communities c ON p.community_did = c.did + LEFT JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did%s WHERE p.deleted_at IS NULL + AND %s %s %s %s ORDER BY %s LIMIT $1 - `, selectClause, timeFilter, cursorFilter, viewerFilter, orderBy) + `, selectClause, visJoin, visWhere, timeFilter, cursorFilter, viewerFilter, orderBy) - // Prepare query arguments + // Prepare query arguments. The viewer DID is bound once at $visibilityParam + // and reused by the block filter above. args := []interface{}{req.Limit + 1} // +1 to check for next page args = append(args, cursorValues...) - args = append(args, viewerArgs...) + args = append(args, req.ViewerDID) // Execute query rows, err := r.db.QueryContext(ctx, query, args...) diff --git a/internal/db/postgres/feed_repo.go b/internal/db/postgres/feed_repo.go index 1c4f0f9..df239cb 100644 --- a/internal/db/postgres/feed_repo.go +++ b/internal/db/postgres/feed_repo.go @@ -46,32 +46,39 @@ func (r *postgresFeedRepo) GetCommunityFeed(ctx context.Context, req communityFe selectClause = feedPostSelectClause("NULL::numeric") } - // Build optional viewer block filter (only when authenticated viewer is present) + // The admission visibility gate always runs, so no read path can serve a + // post its community has not admitted. Its viewer parameter ($visibilityParam) + // carries the read's viewer DID — empty for the anonymous public, which sees + // accepted content only. + visibilityParam := 3 + len(cursorValues) + visJoin, visWhere := visiblePostsJoin(visibilityParam) + + // The viewer block filter reuses that same viewer parameter; it is only + // meaningful for an authenticated viewer and absent for the public. var viewerFilter string - var viewerArgs []interface{} if req.ViewerDID != "" { - viewerParamIdx := 3 + len(cursorValues) - viewerFilter = fmt.Sprintf("AND NOT EXISTS (SELECT 1 FROM user_blocks WHERE blocker_did = $%d AND blocked_did = p.author_did)", viewerParamIdx) - viewerArgs = append(viewerArgs, req.ViewerDID) + viewerFilter = fmt.Sprintf("AND NOT EXISTS (SELECT 1 FROM user_blocks WHERE blocker_did = $%d AND blocked_did = p.author_did)", visibilityParam) } query := fmt.Sprintf(` %s - INNER JOIN users u ON p.author_did = u.did - INNER JOIN communities c ON p.community_did = c.did + LEFT JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did%s WHERE p.community_did = $1 AND p.deleted_at IS NULL + AND %s %s %s %s ORDER BY %s LIMIT $2 - `, selectClause, timeFilter, cursorFilter, viewerFilter, orderBy) + `, selectClause, visJoin, visWhere, timeFilter, cursorFilter, viewerFilter, orderBy) - // Prepare query arguments + // Prepare query arguments. The viewer DID is bound once at $visibilityParam + // and reused by the block filter above. args := []interface{}{req.Community, req.Limit + 1} // +1 to check for next page args = append(args, cursorValues...) - args = append(args, viewerArgs...) + args = append(args, req.ViewerDID) // Execute query rows, err := r.db.QueryContext(ctx, query, args...) diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index a41444e..cd27e06 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -37,13 +37,22 @@ type postgresPostRepo struct { // (upvote_count + bridged_upvote_count, etc.) so federated/bridged content shows the // origin platform's votes. score is already stored inclusive of bridged aggregates, so // it is selected as-is. +// +// author_handle is COALESCEd to the author DID because the users join is a LEFT +// join (a federated author with no users row must not vanish, PRD §5.3), so +// u.handle is NULL for an unindexed author; the comment read path does the same +// (comment_repo.go). a.status and a.acceptance_uri come from the admission LEFT +// join every display query splices in via visiblePostsJoin — they carry the +// per-community admission context an author's own view renders, and are NULL for +// legacy/bridged rows that hold no admission. const postViewSelectColumns = ` p.uri, p.cid, p.rkey, - p.author_did, u.handle as author_handle, u.display_name as author_display_name, u.avatar_cid as author_avatar, u.pds_url as author_pds_url, + p.author_did, COALESCE(u.handle, p.author_did) as author_handle, u.display_name as author_display_name, u.avatar_cid as author_avatar, u.pds_url as author_pds_url, p.community_did, c.handle as community_handle, c.name as community_name, c.avatar_cid as community_avatar, c.pds_url as community_pds_url, p.title, p.content, p.content_facets, p.embed, p.content_labels, p.created_at, p.edited_at, p.indexed_at, - p.upvote_count + p.bridged_upvote_count AS upvote_count, p.downvote_count + p.bridged_downvote_count AS downvote_count, p.score, p.comment_count` + p.upvote_count + p.bridged_upvote_count AS upvote_count, p.downvote_count + p.bridged_downvote_count AS downvote_count, p.score, p.comment_count, + a.status AS admission_status, a.acceptance_uri AS admission_acceptance_uri` // NewPostRepository creates a new PostgreSQL post repository func NewPostRepository(db *sql.DB) posts.Repository { @@ -181,18 +190,26 @@ func (r *postgresPostRepo) GetViewsByURIs(ctx context.Context, uris []string) (m return result, nil } - // Static query: the URI set is bound through a single array parameter (= ANY($1)) - // rather than an interpolated IN list, so the SQL is constant (no fmt.Sprintf, one - // cached query plan regardless of batch size) and the values stay fully parameterized. + // The URI set is bound through a single array parameter (= ANY($1)) rather + // than an interpolated IN list, so the SQL stays fully parameterized and the + // plan is cached regardless of batch size. + // + // post.get is the public permalink surface, so the visibility gate runs with + // an ANONYMOUS viewer ($2 = ""): accepted posts (and legacy/bridged rows) + // only. A pending/rejected/removed post is absent from the result, which the + // service renders as notFoundPost — or, for a removal, upgrades to a + // #removedPost tombstone from the admission row. An author's privileged view + // of their own pending posts is served by actor.getPosts (GetByAuthor), which + // threads a real viewer DID. + visJoin, visWhere := visiblePostsJoin(2) query := ` SELECT` + postViewSelectColumns + ` FROM posts p - INNER JOIN users u ON p.author_did = u.did - INNER JOIN communities c ON p.community_did = c.did - WHERE p.uri = ANY($1) AND p.deleted_at IS NULL - ` + LEFT JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did` + visJoin + ` + WHERE p.uri = ANY($1) AND p.deleted_at IS NULL AND ` + visWhere - rows, err := r.db.QueryContext(ctx, query, pq.Array(uris)) + rows, err := r.db.QueryContext(ctx, query, pq.Array(uris), "") if err != nil { return nil, fmt.Errorf("failed to query posts by URIs: %w", err) } @@ -263,6 +280,19 @@ func (r *postgresPostRepo) GetByAuthor(ctx context.Context, req posts.GetAuthorP paramIndex += len(cursorArgs) } + // The admission visibility gate, threading the viewer DID. A stranger (or the + // anonymous public) sees the author's accepted posts only; the author + // themselves ($viewer = ActorDID) additionally sees their own pending / + // rejected / removed posts, which is how a profile renders per-community + // status (PRD §6.2). This is the alternate-endpoint the feed gate is + // worthless without: an author feed showing ungated content leaks exactly + // what the community feed hides. + visibilityParam := paramIndex + visJoin, visWhere := visiblePostsJoin(visibilityParam) + whereConditions = append(whereConditions, visWhere) + args = append(args, req.ViewerDID) + paramIndex++ + // Add limit to args limit := req.Limit if limit <= 0 { @@ -278,12 +308,12 @@ func (r *postgresPostRepo) GetByAuthor(ctx context.Context, req posts.GetAuthorP query := fmt.Sprintf(` SELECT %s FROM posts p - INNER JOIN users u ON p.author_did = u.did - INNER JOIN communities c ON p.community_did = c.did + LEFT JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did%s WHERE %s ORDER BY p.created_at DESC, p.uri DESC LIMIT $%d - `, postViewSelectColumns, whereClause, paramIndex) + `, postViewSelectColumns, visJoin, whereClause, paramIndex) // Execute query rows, err := r.db.QueryContext(ctx, query, args...) @@ -413,6 +443,8 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er communityHandle sql.NullString communityAvatar sql.NullString communityPDSURL sql.NullString + admissionStatus sql.NullString + acceptanceURI sql.NullString ) dest := []interface{}{ @@ -422,6 +454,7 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er &title, &content, &facets, &embed, &labelsJSON, &postView.CreatedAt, &editedAt, &postView.IndexedAt, &postView.UpvoteCount, &postView.DownvoteCount, &postView.Score, &postView.CommentCount, + &admissionStatus, &acceptanceURI, } dest = append(dest, extraDest...) @@ -462,6 +495,17 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er postView.EditedAt = &editedAt.Time } + // Per-community admission context (PRD §6.2). Present on any row the + // visibility predicate returned that carries an admission decision — every + // accepted post, and an author's own non-accepted posts on their profile. + // Absent (NULL) for legacy/bridged rows, which omit it on the wire. + if admissionStatus.Valid { + postView.Status = admissionStatus.String + } + if acceptanceURI.Valid { + postView.AcceptanceURI = acceptanceURI.String + } + // Parse facets JSON into local variable (will be added to record below) // Log errors but continue - malformed optional fields shouldn't break the response var facetArray []interface{} diff --git a/internal/db/postgres/timeline_repo.go b/internal/db/postgres/timeline_repo.go index 6cb0dd3..7c3de70 100644 --- a/internal/db/postgres/timeline_repo.go +++ b/internal/db/postgres/timeline_repo.go @@ -46,21 +46,28 @@ func (r *postgresTimelineRepo) GetTimeline(ctx context.Context, req timeline.Get selectClause = feedPostSelectClause("NULL::numeric") } + // The admission visibility gate reuses $1 (the subscriber's DID) as the + // viewer — the same intentional reuse the block filter below makes — so a + // pending post reaching the home feed is impossible, while the subscriber + // still sees their own non-accepted posts in communities they subscribe to. + visJoin, visWhere := visiblePostsJoin(1) + // Join with community_subscriptions to get posts from subscribed communities query := fmt.Sprintf(` %s - INNER JOIN users u ON p.author_did = u.did + LEFT JOIN users u ON p.author_did = u.did INNER JOIN communities c ON p.community_did = c.did - INNER JOIN community_subscriptions cs ON p.community_did = cs.community_did + INNER JOIN community_subscriptions cs ON p.community_did = cs.community_did%s WHERE cs.user_did = $1 AND p.deleted_at IS NULL + AND %s -- Intentional $1 reuse: the viewer's DID (cs.user_did) is also the blocker for block filtering AND NOT EXISTS (SELECT 1 FROM user_blocks WHERE blocker_did = $1 AND blocked_did = p.author_did) %s %s ORDER BY %s LIMIT $2 - `, selectClause, timeFilter, cursorFilter, orderBy) + `, selectClause, visJoin, visWhere, timeFilter, cursorFilter, orderBy) // Prepare query arguments args := []interface{}{req.UserDID, req.Limit + 1} // +1 to check for next page diff --git a/internal/db/postgres/user_repo.go b/internal/db/postgres/user_repo.go index f65d3df..c2a8fe7 100644 --- a/internal/db/postgres/user_repo.go +++ b/internal/db/postgres/user_repo.go @@ -236,9 +236,18 @@ func (r *postgresUserRepo) GetProfileStats(ctx context.Context, did string) (*us // Reputation represents historical contributions, while membership_count // reflects current active community access. A banned user keeps their // earned reputation but loses the membership count. + // post_count counts VISIBLE posts only: a profile advertising posts no reader + // can reach is a side channel onto non-accepted content (PRD §6.2). A post + // with a decision counts only once its own community accepted it; a row with + // no admission (legacy, bridged, or an as-yet-unjudged postv2) counts as + // before. This is the public count — the anonymous accepted-or-undecided rule, + // with no author self-view branch — matching visiblePostsJoin. query := ` SELECT - (SELECT COUNT(*) FROM posts WHERE author_did = $1 AND deleted_at IS NULL) as post_count, + (SELECT COUNT(*) FROM posts p + LEFT JOIN community_post_admissions a ON a.community_did = p.community_did AND a.post_uri = p.uri + WHERE p.author_did = $1 AND p.deleted_at IS NULL + AND (a.status = 'accepted' OR a.status IS NULL)) as post_count, (SELECT COUNT(*) FROM comments WHERE commenter_did = $1 AND deleted_at IS NULL) as comment_count, (SELECT COUNT(*) FROM community_subscriptions WHERE user_did = $1) as community_count, (SELECT COUNT(*) FROM community_memberships WHERE user_did = $1 AND is_banned = false) as membership_count, -- 2.51.2 From b51374df5f902f0d630f8c94ce49ac56b6ac7656 Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 08:38:27 -0700 Subject: [PATCH 4/6] =?UTF-8?q?feat(posts):=20GREEN=20cycle=202=20?= =?UTF-8?q?=E2=80=94=20collection-aware=20fail-closed=20+=20getComments=20?= =?UTF-8?q?visibility=20+=20count=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collection-aware fail-closed: a missing admission row is VISIBLE only for non-postv2 collections (legacy community.post / social.coves.post / bridged — all reached the posts table via the old credentialed write-forward path, accepted-by-construction) and HIDDEN for a postv2 without a row (failed/absent pending seed) except to its own author — split_part(p.uri,'/',4) discriminates the collection. getComments header now routes through the same anonymous predicate post.get uses (endpoints can't diverge) + closes the 2026-07-29 soft-delete leak. countAcceptedPostsForCommunity helper landed; the stored-column incrementer reconciliation is deferred to task 8 (no leak meanwhile — every display query already excludes non-accepted). make ci 5147/0. Co-Authored-By: Claude Fable 5 --- internal/core/comments/comment_service.go | 58 ++++++++++++++++++ internal/db/postgres/post_visibility.go | 75 +++++++++++++++-------- internal/db/postgres/user_repo.go | 15 +++-- 3 files changed, 118 insertions(+), 30 deletions(-) diff --git a/internal/core/comments/comment_service.go b/internal/core/comments/comment_service.go index 6a5b445..4ed5cf9 100644 --- a/internal/core/comments/comment_service.go +++ b/internal/core/comments/comment_service.go @@ -161,6 +161,17 @@ func (s *commentService) GetComments(ctx context.Context, req *GetCommentsReques return nil, fmt.Errorf("failed to fetch post: %w", err) } + // 2a. Gate the thread header on the read-path visibility predicate (task 7, + // PRD §6.2). GetByURI is admission-blind and serves soft-deleted rows (the + // 2026-07-29 defect), so without this the thread endpoint hands a non-author + // the full header — title, content, author — of a post the feeds correctly + // hide. The header must answer as post.get does: a soft-deleted post is gone, + // and a postv2 its community has not admitted is root-not-found to everyone + // but its own author. + if err := s.assertRootHeaderVisible(ctx, post, req.ViewerDID); err != nil { + return nil, err + } + // Build post view for response (hydrates author handle and community name) postView := s.buildPostView(ctx, post, req.ViewerDID) @@ -203,6 +214,53 @@ func (s *commentService) GetComments(ctx context.Context, req *GetCommentsReques }, nil } +// assertRootHeaderVisible enforces the read-path visibility predicate on a +// getComments thread header, returning ErrRootNotFound when the post must not be +// shown to this viewer. +// +// It answers through the SAME anonymous predicate post.get uses — the post +// repository's GetViewsByURIs, which is admission- and soft-delete-aware — so the +// two endpoints can never diverge on what the public may read: +// +// - a soft-deleted post is gone (GetByURI still returns it, so the deleted_at +// check is explicit here — this is the half that closes the 2026-07-29 leak); +// - a URI GetViewsByURIs returns is visible to the public (accepted, or a +// legacy/bridged row that predates admissions) → shown; +// - a URI it omits is hidden. For an author-owned postv2 that is the +// fail-closed answer, so a non-author gets ErrRootNotFound; the author still +// reaches their own non-accepted postv2, matching the feed's author branch. +// +// The collection guard is what keeps this correct when postRepo is a unit-test +// fake whose GetViewsByURIs returns nothing: a non-postv2 URI it omits is a fake, +// not a hidden row, so it stays visible. A real hidden non-postv2 cannot occur — +// the predicate never hides a non-postv2 row — so nothing real is leaked. +func (s *commentService) assertRootHeaderVisible(ctx context.Context, post *posts.Post, viewerDID *string) error { + if post.DeletedAt != nil { + return ErrRootNotFound + } + + views, err := s.postRepo.GetViewsByURIs(ctx, []string{post.URI}) + if err != nil { + return fmt.Errorf("failed to check post visibility: %w", err) + } + if _, visible := views[post.URI]; visible { + return nil + } + + // Not served by the anonymous predicate. Fail closed for an author-owned + // postv2 the viewer does not own; every other collection stays visible. + if posts.CollectionOfPostURI(post.URI) == posts.PostV2Collection { + viewer := "" + if viewerDID != nil { + viewer = *viewerDID + } + if viewer != post.AuthorDID { + return ErrRootNotFound + } + } + return nil +} + // getCommentSubtree returns the subtree rooted at the comment identified by req.ParentRkey // The parent comment is the sole top-level entry in the response, with descendants nested // beneath it. Depth is relative to the parent (depth 0 = just the parent comment), sort diff --git a/internal/db/postgres/post_visibility.go b/internal/db/postgres/post_visibility.go index 51a2939..b96f890 100644 --- a/internal/db/postgres/post_visibility.go +++ b/internal/db/postgres/post_visibility.go @@ -4,6 +4,8 @@ import ( "context" "database/sql" "fmt" + + "Coves/internal/core/posts" ) // The centralized read-path visibility predicate (task 7, PRD §6.2). @@ -25,27 +27,33 @@ import ( // post_uri), at most one admission row can match, so the LEFT JOIN never // duplicates a post. // -// # The status rule +// # The status rule is COLLECTION-AWARE and fails closed for postv2 // // The gate turns on the admission row the LEFT JOIN produced for the post's OWN // community: // // - a.status = 'accepted' → visible to everyone. -// - a.status IS NULL → visible. No admission row exists for this -// (community, post): a legacy community-repo post, a bridged post, or any -// other collection that never goes through the admission engine, plus the -// narrow window before the consumer opens a fresh postv2's pending row -// (authorpost.go writes the post and its pending admission in separate -// transactions). These carry no decision to gate on, so they stay visible -// exactly as they were before task 7 — the read path is not the place to -// retro-hide content that predates admissions. +// - a.status IS NULL → the collection decides. No admission row exists +// for this (community, post), and what that MEANS depends on the collection: +// a LEGACY community-repo post (social.coves.community.post), a bridged post, +// or any non-postv2 collection was never routed through the admission engine +// — it is accepted by construction (it lives in the community's own repo) and +// must stay visible until task 8 drains it. But the task-5 consumer ALWAYS +// seeds a pending row the moment it indexes a postv2, so a postv2 with NO row +// is not grandfathered content — the seed failed, and the secure answer is to +// FAIL CLOSED. So a missing row is visible for a non-postv2 collection, and +// hidden for a postv2 (except to its own author). // - a.status IN (pending, pending_reacceptance, removed, rejected) AND the // viewer is the author → visible. An author sees their own posts in every // admission state so a client can render "pending review" / "removed" on the // author's own profile (PRD §6.2). Any OTHER viewer — including the anonymous // public, whose DID is "" — sees accepted content only. This is the security -// core: every non-accepted post that HAS a decision is invisible to -// non-authors on every read path. +// core: every non-accepted post that HAS a decision, and every postv2 with no +// decision, is invisible to non-authors on every read path. +// +// The collection is the fourth '/'-segment of the AT-URI +// (at:////), read with split_part; authorities and +// rkeys carry no '/', so segment 4 is exactly CollectionOfPostURI's answer. // // visiblePostsJoin returns the JOIN clause to splice into the FROM/JOIN section // after `FROM posts p`, and the boolean WHERE fragment to AND into the query's @@ -60,9 +68,9 @@ func visiblePostsJoin(viewerParam int) (joinSQL, whereSQL string) { whereSQL = fmt.Sprintf(`( a.status = 'accepted' - OR a.status IS NULL + OR (a.status IS NULL AND (split_part(p.uri, '/', 4) <> '%s' OR p.author_did = $%d)) OR (a.status IN ('pending', 'pending_reacceptance', 'removed', 'rejected') AND p.author_did = $%d) - )`, viewerParam) + )`, posts.PostV2Collection, viewerParam, viewerParam) return joinSQL, whereSQL } @@ -70,18 +78,37 @@ func visiblePostsJoin(viewerParam int) (joinSQL, whereSQL string) { // countAcceptedPostsForCommunity is the accepted-only source of truth for a // community's post_count (task 7, PRD §6.2). // -// STUB — returns 0 until GREEN implements it. It counts the posts a community has -// ACCEPTED: a join of `posts` to community_post_admissions on the subject key -// with status = 'accepted'. It exists because community.post_count is a STORED -// column whose only incrementer is the old community-repo write path -// (community_repo_memberships.go) — nothing advances it on an acceptance, so it -// is stale under author-owned posts. Whether GREEN recomputes the count live or -// reconciles the stored column from the admission consumer, THIS is the value it -// must converge on. The read paths already exclude non-accepted rows, so this is -// a counting concern, not a content leak — see the cycle-2 report for the -// recommendation to sequence the counter itself as a consumer-side follow-up. +// It counts the posts a community has ACCEPTED: a join of `posts` to +// community_post_admissions on the subject key with status = 'accepted', +// excluding soft-deleted rows. +// +// It exists because community.post_count is a STORED column whose only +// incrementer is the old community-repo write path (community_repo_memberships.go) +// — nothing advances it on an acceptance, so it is stale under author-owned +// posts. THIS is the accepted-only value the counter must converge on. +// +// DEFERRED, deliberately not wired here: reconciling the stored column is a +// consumer-side follow-up (increment on the accept transition, decrement on +// remove/unaccept), sequenced to task 8 — not a read-path concern. There is NO +// content leak in the meantime: every display query already excludes +// non-accepted rows via visiblePostsJoin, so the stale column is a cosmetic +// count, not reachable content. This helper lands the semantics (and its pin, +// TestCommunityPostCountVisibility_AcceptedOnly) now; the incrementer follows. func countAcceptedPostsForCommunity(ctx context.Context, db *sql.DB, communityDID string) (int, error) { - return 0, nil + var count int + err := db.QueryRowContext(ctx, ` + SELECT COUNT(*) + FROM posts p + JOIN community_post_admissions a + ON a.community_did = p.community_did AND a.post_uri = p.uri + WHERE p.community_did = $1 + AND p.deleted_at IS NULL + AND a.status = 'accepted' + `, communityDID).Scan(&count) + if err != nil { + return 0, fmt.Errorf("counting accepted posts for community %s: %w", communityDID, err) + } + return count, nil } // Referenced so the count stub is not flagged as dead before GREEN wires it into diff --git a/internal/db/postgres/user_repo.go b/internal/db/postgres/user_repo.go index c2a8fe7..27055a3 100644 --- a/internal/db/postgres/user_repo.go +++ b/internal/db/postgres/user_repo.go @@ -237,17 +237,20 @@ func (r *postgresUserRepo) GetProfileStats(ctx context.Context, did string) (*us // reflects current active community access. A banned user keeps their // earned reputation but loses the membership count. // post_count counts VISIBLE posts only: a profile advertising posts no reader - // can reach is a side channel onto non-accepted content (PRD §6.2). A post - // with a decision counts only once its own community accepted it; a row with - // no admission (legacy, bridged, or an as-yet-unjudged postv2) counts as - // before. This is the public count — the anonymous accepted-or-undecided rule, - // with no author self-view branch — matching visiblePostsJoin. + // can reach is a side channel onto non-accepted content (PRD §6.2). This is + // the public count — the anonymous, collection-aware rule of visiblePostsJoin, + // with no author self-view branch: a post with a decision counts only once its + // own community accepted it; a row with no admission counts iff it is NOT an + // author-owned postv2 (legacy/bridged stays counted, a postv2 with a missing/ + // failed pending seed does not — fail closed). The collection is the AT-URI's + // fourth '/'-segment (split_part), same as CollectionOfPostURI. query := ` SELECT (SELECT COUNT(*) FROM posts p LEFT JOIN community_post_admissions a ON a.community_did = p.community_did AND a.post_uri = p.uri WHERE p.author_did = $1 AND p.deleted_at IS NULL - AND (a.status = 'accepted' OR a.status IS NULL)) as post_count, + AND (a.status = 'accepted' + OR (a.status IS NULL AND split_part(p.uri, '/', 4) <> 'social.coves.community.postv2'))) as post_count, (SELECT COUNT(*) FROM comments WHERE commenter_did = $1 AND deleted_at IS NULL) as comment_count, (SELECT COUNT(*) FROM community_subscriptions WHERE user_did = $1) as community_count, (SELECT COUNT(*) FROM community_memberships WHERE user_did = $1 AND is_banned = false) as membership_count, -- 2.51.2 From ebd581f83c91dd126e8acbdf0c968d768f0122d9 Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 09:03:47 -0700 Subject: [PATCH 5/6] =?UTF-8?q?test(posts):=20RED=20=E2=80=94=20security-r?= =?UTF-8?q?eview=20batch=20pins=20(P1-P5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two live content leaks plus coverage, all in the read-path visibility suites: - P1 getComments removed-legacy leak: the thread-header gate infers visibility from collection ('omitted by GetViewsByURIs + non-postv2 => show it'), so a moderator-REMOVED legacy community.post (which can carry a removed admission row — applyRemoval has no collection guard) is served to anonymous callers. Pin: removed legacy => ErrRootNotFound; removed postv2 => ErrRootNotFound; soft-deleted LEGACY => ErrRootNotFound (the real 2026-07-29 case, where deleted_at is the sole gate); accepted served; author sees own pending. - P2 accepted branch ignores the pinned CID: visiblePostsJoin gates on status='accepted' without a.accepted_cid = p.cid, so in the edit-> reacceptance window it renders un-attested content under a stale acceptance (§5.5 read-side). Pin: accepted_cid != p.cid => hidden on feed + post.get. - P3 unknown-author LEFT-join guarded on every feed now (was post.get only): an accepted post by an author with no users row appears with a COALESCE'd handle on getCommunity, getDiscover, getTimeline, actor.getPosts. - P5 header rebuild drops hydration: the served getComments header carries the admission context (status/acceptanceUri) — buildPostView must reuse the hydrated view, not rebuild from the raw Post. - P4 (reference-only, SAFE): actor.getComments carries the root as a CommentRef (uri/cid) only, never root content — pinned + reported safe. Co-Authored-By: Claude Fable 5 --- internal/db/postgres/post_visibility_test.go | 254 +++++++++++++++++-- 1 file changed, 231 insertions(+), 23 deletions(-) diff --git a/internal/db/postgres/post_visibility_test.go b/internal/db/postgres/post_visibility_test.go index 98f03ae..1a888be 100644 --- a/internal/db/postgres/post_visibility_test.go +++ b/internal/db/postgres/post_visibility_test.go @@ -518,15 +518,28 @@ func TestGetCommentsVisibility_HeaderIsAdmissionAndDeleteAware(t *testing.T) { createTestUser(t, db, "visgcstranger.test", stranger) base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) - accepted := seedVisibilityPost(t, db, community, author, "gcacc", "accepted header", base.Add(3*time.Hour)) - pending := seedVisibilityPost(t, db, community, author, "gcpen", "pending header", base.Add(2*time.Hour)) - deleted := seedVisibilityPost(t, db, community, author, "gcdel", "deleted header", base.Add(1*time.Hour)) - seedVisibilityAdmission(t, db, community, accepted, posts.AdmissionStatusAccepted, "bafypostv2gcacc", "") - seedVisibilityAdmission(t, db, community, pending, posts.AdmissionStatusPending, "", "") - seedVisibilityAdmission(t, db, community, deleted, posts.AdmissionStatusAccepted, "bafypostv2gcdel", "") - // Soft-delete the third post the way the consumer does. - _, err := db.ExecContext(ctx, `UPDATE posts SET deleted_at = NOW() WHERE uri = $1`, deleted) + acceptedV2 := seedVisibilityPost(t, db, community, author, "gcacc", "accepted header", base.Add(6*time.Hour)) + pendingV2 := seedVisibilityPost(t, db, community, author, "gcpen", "pending header", base.Add(5*time.Hour)) + removedV2 := seedVisibilityPost(t, db, community, author, "gcrv2", "removed postv2 header", base.Add(4*time.Hour)) + seedVisibilityAdmission(t, db, community, acceptedV2, posts.AdmissionStatusAccepted, "bafypostv2gcacc", "") + seedVisibilityAdmission(t, db, community, pendingV2, posts.AdmissionStatusPending, "", "") + seedVisibilityAdmission(t, db, community, removedV2, posts.AdmissionStatusRemoved, "", "rule-violation") + + // A LEGACY community.post carrying a REMOVED admission row. A legacy post can + // hold one — applyRemoval has no collection guard (unlike applyAcceptance) — so + // a moderator removes a legacy post exactly as they remove a postv2. This is + // the P1 leak: GetViewsByURIs correctly omits it, but a header gate that infers + // "omitted + non-postv2 = show it" serves its content to the anonymous public. + removedLegacy := seedFilterablePost(t, db, community, author, "gcrleg", base.Add(3*time.Hour)) + seedVisibilityAdmission(t, db, community, removedLegacy, posts.AdmissionStatusRemoved, "", "rule-violation") + + // A LEGACY community.post that has been SOFT-DELETED, with no admission row. + // This is the REAL 2026-07-29 case: nothing but deleted_at hides it (a legacy + // post with no admission is otherwise visible), so it isolates the deleted_at + // gate that GetByURI omits. + deletedLegacy := seedFilterablePost(t, db, community, author, "gcdleg", base.Add(2*time.Hour)) + _, err := db.ExecContext(ctx, `UPDATE posts SET deleted_at = NOW() WHERE uri = $1`, deletedLegacy) require.NoError(t, err) service := comments.NewCommentServiceWithPDSFactory( @@ -546,29 +559,60 @@ func TestGetCommentsVisibility_HeaderIsAdmissionAndDeleteAware(t *testing.T) { return service.GetComments(ctx, &comments.GetCommentsRequest{PostURI: postURI, ViewerDID: viewer}) } - t.Run("an accepted post serves its header", func(t *testing.T) { - resp, err := header(t, accepted, stranger) + requireHidden := func(t *testing.T, postURI, viewer, what string) { + t.Helper() + _, err := header(t, postURI, viewer) + require.Errorf(t, err, "getComments served %s", what) + assert.ErrorIsf(t, err, comments.ErrRootNotFound, + "%s must be root-not-found through getComments, the same answer post.get gives", what) + } + + t.Run("an accepted post serves its header, carrying the hydrated admission context", func(t *testing.T) { + resp, err := header(t, acceptedV2, stranger) require.NoError(t, err, "getComments must serve the header of an accepted post") require.NotNil(t, resp.Post) postView, ok := resp.Post.(*posts.PostView) require.Truef(t, ok, "getComments post header is %T, not *posts.PostView", resp.Post) - assert.Equal(t, accepted, postView.URI) + assert.Equal(t, acceptedV2, postView.URI) + + // P5: the header the visibility check hydrated (via GetViewsByURIs) must be + // the one served — not a second view rebuilt from the raw Post, which drops + // the admission context. status and acceptanceUri are the fields postView + // gained for exactly this (PRD §6.2); a thread header that omits them makes + // the thread endpoint disagree with post.get about a post they both serve. + assert.Equalf(t, string(posts.AdmissionStatusAccepted), postView.Status, + "the served thread header dropped the admission status. buildPostView rebuilds the view from the raw Post "+ + "and discards the admission-aware view GetViewsByURIs already hydrated — reuse that view instead") + assert.NotEmptyf(t, postView.AcceptanceURI, + "the served thread header dropped acceptanceUri — a client cannot follow the accepted post to its attestation") + }) + + t.Run("the author sees their own pending post's header", func(t *testing.T) { + resp, err := header(t, pendingV2, author) + require.NoError(t, err, "an author must reach their own pending post's thread header, matching the feed's author branch") + require.NotNil(t, resp.Post) + }) + + t.Run("a pending postv2 header is hidden from a non-author", func(t *testing.T) { + requireHidden(t, pendingV2, stranger, "a non-author the header of a PENDING postv2") }) - t.Run("a pending post's header is hidden from a non-author", func(t *testing.T) { - _, err := header(t, pending, stranger) - require.Errorf(t, err, "getComments served a non-author the header of a PENDING post — the alternate-endpoint "+ - "leak PRD §6.2 names: a post hidden from the feed is fully readable through its comment thread") - assert.ErrorIs(t, err, comments.ErrRootNotFound, - "a pending post must be root-not-found to a non-author's getComments, the same answer post.get gives") + t.Run("a removed postv2 header is hidden from a non-author", func(t *testing.T) { + requireHidden(t, removedV2, stranger, "a non-author the header of a REMOVED postv2") }) - t.Run("a soft-deleted post no longer leaks (closes the 2026-07-29 defect)", func(t *testing.T) { - _, err := header(t, deleted, stranger) - require.Errorf(t, err, "getComments served the full header of a SOFT-DELETED post. GetByURI has no deleted_at "+ - "filter, so the withdrawn post's title/content/author are still returned through the thread endpoint — the "+ - "defect filed 2026-07-29") - assert.ErrorIs(t, err, comments.ErrRootNotFound) + t.Run("a removed LEGACY post's header is hidden from a non-author (P1 leak)", func(t *testing.T) { + // The header gate must not infer visibility from the collection. A legacy + // post with a removed admission row is omitted by GetViewsByURIs for a REAL + // reason, and treating "omitted + non-postv2" as "show it" serves a + // moderator-removed post's content to anyone. + requireHidden(t, removedLegacy, stranger, + "a non-author the content of a moderator-REMOVED legacy community.post (the collection-inference leak)") + }) + + t.Run("a soft-deleted LEGACY post no longer leaks (the real 2026-07-29 case)", func(t *testing.T) { + requireHidden(t, deletedLegacy, stranger, + "a non-author the header of a SOFT-DELETED legacy post — nothing but deleted_at hides it, and GetByURI has no such filter") }) } @@ -611,3 +655,167 @@ func TestCommunityPostCountVisibility_AcceptedOnly(t *testing.T) { "a community's accepted-post count must be 1 (3 seeded: accepted, pending, removed); a count that includes "+ "non-accepted rows advertises content no reader can reach") } + +// TestPostGetVisibility_AcceptedBranchHonorsPinnedCID pins §5.5 at the READ path: +// an acceptance pins a CID, and edited content must never render under it. The +// admission consumer commits an edit's new content (posts.cid advances) in a +// SEPARATE transaction from the admission transition (accepted → +// pending_reacceptance), so there is a window where the row is still +// status='accepted' but posts.cid no longer equals accepted_cid. In that window +// the accepted content the community attested to is GONE and the new, +// un-attested content stands in its place — and a predicate that gates on +// status='accepted' alone renders it. +// +// The gate must also check a.accepted_cid = p.cid. This seeds the mismatch +// directly (accepted_cid one value, the post's cid another) rather than racing +// the consumer. +func TestPostGetVisibility_AcceptedBranchHonorsPinnedCID(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "pc") + author := "did:plc:vispcauthor" + createTestUser(t, db, "vispcauthor.test", author) + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + + // The post's real content CID is "bafypostv2pcmis" (seedVisibilityPost derives + // it from the rkey). The acceptance pins a DIFFERENT, older CID. + mismatched := seedVisibilityPost(t, db, community, author, "pcmis", "edited past the acceptance", base.Add(2*time.Hour)) + seedVisibilityAdmission(t, db, community, mismatched, posts.AdmissionStatusAccepted, "bafySTALEacceptedcid", "") + + // A control whose acceptance pins the CURRENT content CID. + matched := seedVisibilityPost(t, db, community, author, "pcmat", "still matches the acceptance", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, matched, posts.AdmissionStatusAccepted, "bafypostv2pcmat", "") + + postRepo := NewPostRepository(db) + views, err := postRepo.GetViewsByURIs(ctx, []string{mismatched, matched}) + require.NoError(t, err) + + assert.Containsf(t, views, matched, "an accepted post whose pinned CID still matches its content must be visible") + assert.NotContainsf(t, views, mismatched, + "post.get served an 'accepted' post whose pinned CID no longer matches its content. The acceptance attests to "+ + "accepted_cid, and posts.cid has moved past it (an edit landed before the pending_reacceptance transition) — "+ + "rendering it shows un-attested content under a stale acceptance (§5.5). The accepted branch must AND "+ + "a.accepted_cid = p.cid.") + + feedRepo := NewCommunityFeedRepository(db, "test-secret") + feed, _, err := feedRepo.GetCommunityFeed(ctx, communityFeeds.GetCommunityFeedRequest{ + Community: community, ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50, + }) + require.NoError(t, err) + got := feedURIs(feed) + assert.Contains(t, got, matched) + assert.NotContainsf(t, got, mismatched, + "the community feed rendered an accepted post whose content has drifted past its pinned CID (§5.5 read-side leak)") +} + +// TestFeedsVisibility_UnknownAuthorAccepted guards the §5.3 open-posting promise +// on EVERY feed, not just post.get. An accepted post by an author with no `users` +// row (a federated author the AppView has not hydrated) must appear, with its +// handle COALESCE'd to the author DID. The users join on each feed is a LEFT join +// for exactly this — reverting any one of them to INNER would silently vanish +// every federated author's accepted post, and cycle 1 pinned this on post.get +// alone. These are the per-feed regression guards. +func TestFeedsVisibility_UnknownAuthorAccepted(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "ua") + // Deliberately NO createTestUser: this author is indexed nowhere. + unknownAuthor := "did:plc:visuaunknownfederated" + + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + post := seedVisibilityPost(t, db, community, unknownAuthor, "uapost", "federated accepted", base.Add(1*time.Hour)) + seedVisibilityAdmission(t, db, community, post, posts.AdmissionStatusAccepted, "bafypostv2uapost", "") + + assertUnknownAuthorHandle := func(t *testing.T, pv *posts.PostView) { + t.Helper() + require.NotNil(t, pv.Author) + assert.Equalf(t, unknownAuthor, pv.Author.Handle, + "an unindexed author's handle must COALESCE to their DID; a NULL handle means the feed is INNER-joining users "+ + "and dropping every federated author's accepted post (§5.3)") + } + + t.Run("feed.getCommunity", func(t *testing.T) { + feed, _, err := NewCommunityFeedRepository(db, "test-secret").GetCommunityFeed(ctx, + communityFeeds.GetCommunityFeedRequest{Community: community, ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50}) + require.NoError(t, err) + require.Containsf(t, feedURIs(feed), post, "the community feed dropped an accepted post by an unindexed author (users INNER join?)") + assertUnknownAuthorHandle(t, feed[0].Post) + }) + + t.Run("feed.getDiscover", func(t *testing.T) { + feed, _, err := NewDiscoverRepository(db, "test-secret").GetDiscover(ctx, + discover.GetDiscoverRequest{ViewerDID: publicViewer, Sort: visibilitySort, Limit: 50}) + require.NoError(t, err) + require.Containsf(t, discoverFeedURIs(feed), post, "discover dropped an accepted post by an unindexed author") + assertUnknownAuthorHandle(t, feed[0].Post) + }) + + t.Run("feed.getTimeline", func(t *testing.T) { + subscriber := "did:plc:visuasubscriber" + createTestUser(t, db, "visuasubscriber.test", subscriber) + _, err := db.ExecContext(ctx, `INSERT INTO community_subscriptions (user_did, community_did, subscribed_at) VALUES ($1, $2, NOW())`, subscriber, community) + require.NoError(t, err) + feed, _, err := NewTimelineRepository(db, "test-secret").GetTimeline(ctx, + timeline.GetTimelineRequest{UserDID: subscriber, Sort: visibilitySort, Limit: 50}) + require.NoError(t, err) + require.Containsf(t, timelineFeedURIs(feed), post, "the timeline dropped an accepted post by an unindexed author") + assertUnknownAuthorHandle(t, feed[0].Post) + }) + + t.Run("actor.getPosts", func(t *testing.T) { + views, _, err := NewPostRepository(db).GetByAuthor(ctx, posts.GetAuthorPostsRequest{ActorDID: unknownAuthor, Limit: 50}) + require.NoError(t, err) + require.Lenf(t, views, 1, "actor.getPosts dropped an accepted post by an unindexed author") + assert.Equal(t, post, views[0].URI) + assertUnknownAuthorHandle(t, views[0]) + }) +} + +// TestActorCommentsVisibility_RootIsReferenceOnly pins P4's finding: GetActorComments +// carries only a REFERENCE to each comment's root post (root_uri / root_cid), never +// the root's hydrated content — so a comment on a pending or removed root leaks +// nothing about that root, and following the reference lands on the gated post.get. +// The comment itself is the actor's own public speech and is still listed; the root +// being hidden does not suppress it. This is the reference-only guarantee that keeps +// actor.getComments off the leak list. +func TestActorCommentsVisibility_RootIsReferenceOnly(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + ctx := context.Background() + + community := visibilityCommunity(t, db, "ac") + actor := "did:plc:visacactor" + createTestUser(t, db, "visacactor.test", actor) + + // The root is a PENDING postv2 — hidden from the public everywhere else. + root := seedVisibilityPost(t, db, community, actor, "acroot", "secret pending root", time.Now().Add(-time.Hour)) + seedVisibilityAdmission(t, db, community, root, posts.AdmissionStatusPending, "", "") + + commentURI := "at://" + actor + "/social.coves.community.comment/accmt1" + _, err := db.ExecContext(ctx, ` + INSERT INTO comments (uri, cid, rkey, commenter_did, root_uri, root_cid, parent_uri, parent_cid, content, created_at) + VALUES ($1, $2, $3, $4, $5, $6, $5, $6, $7, NOW()) + `, commentURI, "bafycmtac", "accmt1", actor, root, "bafypostv2acroot", "a comment on a hidden root") + require.NoError(t, err) + + service := comments.NewCommentServiceWithPDSFactory( + NewCommentRepository(db), NewUserRepository(db), NewPostRepository(db), NewCommunityRepository(db), nil, nil, + ) + + resp, err := service.GetActorComments(ctx, &comments.GetActorCommentsRequest{ActorDID: actor, Limit: 50}) + require.NoError(t, err) + require.Lenf(t, resp.Comments, 1, "the actor's comment must be listed even though its root is hidden — the comment is the actor's own public record") + + cv := resp.Comments[0] + require.Equalf(t, commentURI, cv.URI, "the listed comment must be the actor's own comment") + require.NotNil(t, cv.Post, "the comment must reference its root") + assert.Equalf(t, root, cv.Post.URI, "actor.getComments must carry the root as a reference (uri/cid), which following lands on the gated post.get") + // The response shape carries no root-content field at all: the guarantee is + // structural — CommentView holds the root only as a CommentRef (uri/cid), so + // the root's title/body cannot appear here even when it is a hidden post. +} -- 2.51.2 From bcb4e04368c28c474ff3d2cc2d5bc4a3bb143dce Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 09:22:40 -0700 Subject: [PATCH 6/6] =?UTF-8?q?feat(posts):=20GREEN=20=E2=80=94=20task=207?= =?UTF-8?q?=20security-review=20batch=20(P1/P2/P5/P6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compensating control's own hardening pass — two live read-path leaks plus fold-ins, against RED's ebd581f pins. - P1 (getComments removed-legacy leak): the thread-header gate no longer infers visibility from collection (which served a moderator-REMOVED legacy community.post to anonymous callers — a legacy row CAN carry a removed admission). It now consults a REAL admission lookup via the new viewer-aware postgresPostRepo.VisibleHeaderView, reached through an optional type assertion so unit-test fakes degrade to the soft-delete gate. Removed legacy/postv2 and soft-deleted legacy → ErrRootNotFound; accepted served; author sees own pending. - P2 (§5.5 accepted-CID read leak): visiblePostsJoin's accepted branch now ANDs a.accepted_cid = p.cid, so an edited-past-the-acceptance post is hidden on every feed + post.get during the pre-reacceptance window instead of rendering un-attested content under a stale acceptance. - P5 (dropped hydration): the served getComments header is the admission- hydrated view VisibleHeaderView returns (status/acceptanceUri survive), not a second view rebuilt from the raw Post. - P6 (fold-ins, no pins): user post_count + countAcceptedPostsForCommunity honor the pinned CID and share posts.PostV2Collection (killed the raw literal); removedMarkers gained a soft-delete guard (deleted+removed → notFound, not #removedPost) and a single batched GetByPostURIs lookup replacing the per-URI admissions N+1. - P3/P4 already passed as regression guards — untouched. make ci: 5157 passed, 0 failed, 0 skipped. No test files modified. Co-Authored-By: Claude Fable 5 --- internal/core/comments/comment_service.go | 100 +++++++++++----------- internal/core/posts/service.go | 39 +++++++-- internal/db/postgres/post_repo.go | 51 +++++++++++ internal/db/postgres/post_visibility.go | 14 ++- internal/db/postgres/user_repo.go | 19 ++-- 5 files changed, 156 insertions(+), 67 deletions(-) diff --git a/internal/core/comments/comment_service.go b/internal/core/comments/comment_service.go index 4ed5cf9..57b1686 100644 --- a/internal/core/comments/comment_service.go +++ b/internal/core/comments/comment_service.go @@ -161,20 +161,20 @@ func (s *commentService) GetComments(ctx context.Context, req *GetCommentsReques return nil, fmt.Errorf("failed to fetch post: %w", err) } - // 2a. Gate the thread header on the read-path visibility predicate (task 7, - // PRD §6.2). GetByURI is admission-blind and serves soft-deleted rows (the - // 2026-07-29 defect), so without this the thread endpoint hands a non-author - // the full header — title, content, author — of a post the feeds correctly - // hide. The header must answer as post.get does: a soft-deleted post is gone, - // and a postv2 its community has not admitted is root-not-found to everyone - // but its own author. - if err := s.assertRootHeaderVisible(ctx, post, req.ViewerDID); err != nil { + // 2a. Resolve the thread header through the read-path visibility predicate + // (task 7, PRD §6.2). GetByURI is admission-blind and serves soft-deleted rows + // (the 2026-07-29 defect), so without this the thread endpoint hands a + // non-author the full header — title, content, author — of a post the feeds + // correctly hide. The header answers as post.get does: a soft-deleted post is + // gone, and a post its community has not admitted (a pending/removed/rejected + // row on ANY collection, or a failed-seed postv2) is root-not-found to everyone + // but its own author. The resolved view is the admission-hydrated one, so its + // status/acceptanceUri survive onto the served header. + postView, err := s.resolveVisibleHeader(ctx, post, req.ViewerDID) + if err != nil { return nil, err } - // Build post view for response (hydrates author handle and community name) - postView := s.buildPostView(ctx, post, req.ViewerDID) - // 2b. If a parent comment rkey is provided, return only that comment's subtree if req.ParentRkey != "" { return s.getCommentSubtree(ctx, req, postView) @@ -214,51 +214,55 @@ func (s *commentService) GetComments(ctx context.Context, req *GetCommentsReques }, nil } -// assertRootHeaderVisible enforces the read-path visibility predicate on a -// getComments thread header, returning ErrRootNotFound when the post must not be -// shown to this viewer. -// -// It answers through the SAME anonymous predicate post.get uses — the post -// repository's GetViewsByURIs, which is admission- and soft-delete-aware — so the -// two endpoints can never diverge on what the public may read: +// postHeaderVisibilityChecker is the viewer-aware, admission-aware slice of the +// post repository the thread-header gate needs. The real postgres repository +// implements it (VisibleHeaderView); unit-test fakes do not, and a service built +// on a fake keeps the pre-admission behavior — see resolveVisibleHeader. +type postHeaderVisibilityChecker interface { + // VisibleHeaderView returns the hydrated post view iff the post is visible to + // viewerDID under the read-path predicate, or nil when it is hidden. + VisibleHeaderView(ctx context.Context, uri, viewerDID string) (*posts.PostView, error) +} + +// resolveVisibleHeader returns the post view to render as the getComments thread +// header, or ErrRootNotFound when the post must not be shown to this viewer. // -// - a soft-deleted post is gone (GetByURI still returns it, so the deleted_at -// check is explicit here — this is the half that closes the 2026-07-29 leak); -// - a URI GetViewsByURIs returns is visible to the public (accepted, or a -// legacy/bridged row that predates admissions) → shown; -// - a URI it omits is hidden. For an author-owned postv2 that is the -// fail-closed answer, so a non-author gets ErrRootNotFound; the author still -// reaches their own non-accepted postv2, matching the feed's author branch. +// It consults a REAL admission lookup, never the collection: the previous gate +// inferred visibility from the URI's collection, which leaked a moderator-REMOVED +// legacy community.post (a legacy row CAN carry a removed admission — applyRemoval +// has no collection guard). VisibleHeaderView runs the same predicate as post.get +// and the feeds — admission status, pinned CID, soft-delete, and the author +// self-view branch — and returns the admission-hydrated view, so the served +// header carries status/acceptanceUri instead of a second view rebuilt from the +// raw Post (which dropped them). // -// The collection guard is what keeps this correct when postRepo is a unit-test -// fake whose GetViewsByURIs returns nothing: a non-postv2 URI it omits is a fake, -// not a hidden row, so it stays visible. A real hidden non-postv2 cannot occur — -// the predicate never hides a non-postv2 row — so nothing real is leaked. -func (s *commentService) assertRootHeaderVisible(ctx context.Context, post *posts.Post, viewerDID *string) error { - if post.DeletedAt != nil { - return ErrRootNotFound - } - - views, err := s.postRepo.GetViewsByURIs(ctx, []string{post.URI}) - if err != nil { - return fmt.Errorf("failed to check post visibility: %w", err) - } - if _, visible := views[post.URI]; visible { - return nil - } - - // Not served by the anonymous predicate. Fail closed for an author-owned - // postv2 the viewer does not own; every other collection stays visible. - if posts.CollectionOfPostURI(post.URI) == posts.PostV2Collection { +// A unit-test fake postRepo does not implement the checker; such a service keeps +// the pre-admission behavior, gated only on the soft-delete the raw Post carries. +// This is safe because a fake is never wired to real admission data — production +// always uses the real repository, which always implements the checker. +func (s *commentService) resolveVisibleHeader(ctx context.Context, post *posts.Post, viewerDID *string) (*posts.PostView, error) { + if checker, ok := s.postRepo.(postHeaderVisibilityChecker); ok { viewer := "" if viewerDID != nil { viewer = *viewerDID } - if viewer != post.AuthorDID { - return ErrRootNotFound + view, err := checker.VisibleHeaderView(ctx, post.URI, viewer) + if err != nil { + return nil, fmt.Errorf("checking post header visibility: %w", err) + } + if view == nil { + return nil, ErrRootNotFound } + return view, nil } - return nil + + // Fake repository (unit tests): no admission-aware fetch available. Honor only + // the gate the raw Post carries — a soft delete — and build the header from it + // as before. + if post.DeletedAt != nil { + return nil, ErrRootNotFound + } + return s.buildPostView(ctx, post, viewerDID), nil } // getCommentSubtree returns the subtree rooted at the comment identified by req.ParentRkey diff --git a/internal/core/posts/service.go b/internal/core/posts/service.go index 62f6f9d..111d3f8 100644 --- a/internal/core/posts/service.go +++ b/internal/core/posts/service.go @@ -1202,7 +1202,10 @@ func (s *postService) removedMarkers(ctx context.Context, uris []string, views m return markers } + // Collect the absent URIs once (deduped), then resolve their admissions in a + // single batched lookup rather than one round-trip per URI. seen := make(map[string]struct{}, len(uris)) + absent := make([]string, 0, len(uris)) for _, uri := range uris { if views[uri] != nil { continue // visible — not a candidate for a tombstone @@ -1211,25 +1214,43 @@ func (s *postService) removedMarkers(ctx context.Context, uris []string, views m continue } seen[uri] = struct{}{} + absent = append(absent, uri) + } + if len(absent) == 0 { + return markers + } + + admissionsByURI, err := s.admissions.GetByPostURIs(ctx, absent) + if err != nil { + return markers // best-effort: on failure every absent URI stays a plain notFound + } + for _, uri := range absent { // A removal is an admission-state change, not a soft delete, so the post - // row still stands and its own community — the key the admission is - // scoped by — comes straight off it. A URI with no row is genuinely - // not-indexed and stays a notFound. + // row still stands and its own community — the key the admission is scoped + // by — comes straight off it. A URI with no row is genuinely not-indexed + // and stays a notFound. post, err := s.repo.GetByURI(ctx, uri) if err != nil { continue } - admission, err := s.admissions.Get(ctx, post.CommunityDID, uri) - if err != nil || admission == nil || admission.Status != AdmissionStatusRemoved { + // A soft-deleted post is GONE, not a tombstone: the author withdrew it, so + // even a standing removal must render as notFound rather than advertising + // a moderation reason for a post its own author took down. + if post.DeletedAt != nil { continue } - code := "" - if admission.DecisionCode != nil { - code = *admission.DecisionCode + for _, admission := range admissionsByURI[uri] { + if admission.CommunityDID == post.CommunityDID && admission.Status == AdmissionStatusRemoved { + code := "" + if admission.DecisionCode != nil { + code = *admission.DecisionCode + } + markers[uri] = code + break + } } - markers[uri] = code } return markers } diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index cd27e06..55c1c49 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -234,6 +234,57 @@ func (r *postgresPostRepo) GetViewsByURIs(ctx context.Context, uris []string) (m return result, nil } +// VisibleHeaderView returns the hydrated view of a single post IFF it is visible +// to viewerDID under the read-path predicate, and nil when it is hidden (or does +// not exist). +// +// It is the viewer-aware companion to GetViewsByURIs, which GetViewsByURIs itself +// cannot be because posts.Repository's signature is frozen at two arguments +// (three in-suite fakes implement it). getComments needs both halves the +// anonymous batch fetch cannot give it — a VIEWER (so an author reaches their own +// pending post's thread header) and the HYDRATED view (so the served header +// carries status/acceptanceUri, PRD §6.2). It is deliberately NOT on the +// Repository interface: the comment service reaches it by an optional type +// assertion, so a unit-test fake without it degrades gracefully rather than +// forcing every fake to grow a method. +// +// It runs the SAME predicate as every other display query — visiblePostsJoin, +// admission status + pinned-CID + collection fail-closed + author self-view — and +// the same deleted_at filter and scanPostView hydration, so the thread header can +// never diverge from post.get or the feeds on what is visible. +func (r *postgresPostRepo) VisibleHeaderView(ctx context.Context, uri, viewerDID string) (*posts.PostView, error) { + visJoin, visWhere := visiblePostsJoin(2) + query := ` + SELECT` + postViewSelectColumns + ` + FROM posts p + LEFT JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did` + visJoin + ` + WHERE p.uri = $1 AND p.deleted_at IS NULL AND ` + visWhere + + rows, err := r.db.QueryContext(ctx, query, uri, viewerDID) + if err != nil { + return nil, fmt.Errorf("failed to query visible post header: %w", err) + } + defer func() { + if err := rows.Close(); err != nil { + slog.Warn("failed to close rows", "error", err) + } + }() + + if !rows.Next() { + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating visible post header: %w", err) + } + return nil, nil + } + + postView, err := scanPostView(rows) + if err != nil { + return nil, fmt.Errorf("failed to scan visible post header: %w", err) + } + return postView, nil +} + // GetByAuthor retrieves posts by author with filtering and pagination // Supports filter options: posts_with_replies (default), posts_no_replies, posts_with_media // Uses cursor-based pagination with created_at + uri for stable ordering diff --git a/internal/db/postgres/post_visibility.go b/internal/db/postgres/post_visibility.go index b96f890..bbf4b27 100644 --- a/internal/db/postgres/post_visibility.go +++ b/internal/db/postgres/post_visibility.go @@ -32,7 +32,16 @@ import ( // The gate turns on the admission row the LEFT JOIN produced for the post's OWN // community: // -// - a.status = 'accepted' → visible to everyone. +// - a.status = 'accepted' AND a.accepted_cid = p.cid → visible to everyone. +// The CID equality is the §5.5 read-side guard: an acceptance attests to a +// SPECIFIC content CID, and the admission consumer commits an edit's new +// content (posts.cid advances) in a separate transaction from the accepted → +// pending_reacceptance status transition. In that window the row still reads +// 'accepted' while posts.cid has moved past accepted_cid, so the attested +// content is gone and un-attested content stands in its place. Gating on +// status alone would render it; requiring accepted_cid = p.cid hides the +// drifted post from EVERYONE until the status catches up (whereupon the +// author-branch below shows the author their own pending_reacceptance). // - a.status IS NULL → the collection decides. No admission row exists // for this (community, post), and what that MEANS depends on the collection: // a LEGACY community-repo post (social.coves.community.post), a bridged post, @@ -67,7 +76,7 @@ func visiblePostsJoin(viewerParam int) (joinSQL, whereSQL string) { ON a.community_did = p.community_did AND a.post_uri = p.uri` whereSQL = fmt.Sprintf(`( - a.status = 'accepted' + (a.status = 'accepted' AND a.accepted_cid = p.cid) OR (a.status IS NULL AND (split_part(p.uri, '/', 4) <> '%s' OR p.author_did = $%d)) OR (a.status IN ('pending', 'pending_reacceptance', 'removed', 'rejected') AND p.author_did = $%d) )`, posts.PostV2Collection, viewerParam, viewerParam) @@ -104,6 +113,7 @@ func countAcceptedPostsForCommunity(ctx context.Context, db *sql.DB, communityDI WHERE p.community_did = $1 AND p.deleted_at IS NULL AND a.status = 'accepted' + AND a.accepted_cid = p.cid `, communityDID).Scan(&count) if err != nil { return 0, fmt.Errorf("counting accepted posts for community %s: %w", communityDID, err) diff --git a/internal/db/postgres/user_repo.go b/internal/db/postgres/user_repo.go index 27055a3..4faea8b 100644 --- a/internal/db/postgres/user_repo.go +++ b/internal/db/postgres/user_repo.go @@ -1,6 +1,7 @@ package postgres import ( + "Coves/internal/core/posts" "Coves/internal/core/users" "context" "database/sql" @@ -240,22 +241,24 @@ func (r *postgresUserRepo) GetProfileStats(ctx context.Context, did string) (*us // can reach is a side channel onto non-accepted content (PRD §6.2). This is // the public count — the anonymous, collection-aware rule of visiblePostsJoin, // with no author self-view branch: a post with a decision counts only once its - // own community accepted it; a row with no admission counts iff it is NOT an - // author-owned postv2 (legacy/bridged stays counted, a postv2 with a missing/ - // failed pending seed does not — fail closed). The collection is the AT-URI's - // fourth '/'-segment (split_part), same as CollectionOfPostURI. - query := ` + // own community accepted it AND the acceptance still pins the current content + // (§5.5 — a drifted-accepted post is un-attested and does not count); a row + // with no admission counts iff it is NOT an author-owned postv2 (legacy/bridged + // stays counted, a postv2 with a missing/failed pending seed does not — fail + // closed). The collection is the AT-URI's fourth '/'-segment (split_part), same + // as CollectionOfPostURI; the postv2 collection literal is the shared constant. + query := fmt.Sprintf(` SELECT (SELECT COUNT(*) FROM posts p LEFT JOIN community_post_admissions a ON a.community_did = p.community_did AND a.post_uri = p.uri WHERE p.author_did = $1 AND p.deleted_at IS NULL - AND (a.status = 'accepted' - OR (a.status IS NULL AND split_part(p.uri, '/', 4) <> 'social.coves.community.postv2'))) as post_count, + AND ((a.status = 'accepted' AND a.accepted_cid = p.cid) + OR (a.status IS NULL AND split_part(p.uri, '/', 4) <> '%s'))) as post_count, (SELECT COUNT(*) FROM comments WHERE commenter_did = $1 AND deleted_at IS NULL) as comment_count, (SELECT COUNT(*) FROM community_subscriptions WHERE user_did = $1) as community_count, (SELECT COUNT(*) FROM community_memberships WHERE user_did = $1 AND is_banned = false) as membership_count, (SELECT COALESCE(SUM(reputation_score), 0) FROM community_memberships WHERE user_did = $1) as reputation - ` + `, posts.PostV2Collection) stats := &users.ProfileStats{} err := r.db.QueryRowContext(ctx, query, did).Scan( -- 2.51.2