From d80062f5ff09dfe8a9a903404c03c6f67034ff6d Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 10 Sep 2026 07:54:49 +0000 Subject: [PATCH 1/2] feat(curation desk): forward the roster write routes The roster becomes the one place a curator is added or removed (ecency/esync-py#59), so the desk needs to reach it: roster-list, roster-set and roster-retire join the existing desk writes and go through the same fence, the validated username and a key whitelist, so a caller can never forge who is asking. The cached GET roster route is untouched. The private view is a POST because it carries notes, added_by and retired rows, and none of that may enter a body with an s-maxage. rules is the one object here that is refused rather than trimmed. A read filter with an unknown value is dropped so the backend applies its default; an admin setting a rule must not be told it was saved when it was discarded. Closes #100 --- .../CurationDeskPayloadTests.cs | 74 +++++++++++++++++ .../Handlers/PrivateApi.CurationDesk.cs | 83 +++++++++++++++++++ dotnet/EcencyApi/Handlers/Routes.cs | 3 + 3 files changed, 160 insertions(+) diff --git a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs index cf05fbbb..677635f0 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs @@ -16,6 +16,7 @@ public class CurationDeskPayloadTests CurationDeskWrites.RosterFeed, CurationDeskWrites.Tick, CurationDeskWrites.Mark, CurationDeskWrites.MarkClear, CurationDeskWrites.Marks, CurationDeskWrites.Cursor, CurationDeskWrites.RecommendMeta, CurationDeskWrites.RecommendationDismiss, CurationDeskWrites.Ingest, + CurationDeskWrites.RosterList, CurationDeskWrites.RosterSet, CurationDeskWrites.RosterRetire, }; private const string IngestBody = @@ -54,6 +55,10 @@ private static string ValidBodyFor(CurationDeskWrites.Route route) return "{" + forged + "\"author\":\"bob\",\"permlink\":\"p\",\"action\":\"restore\"}"; if (ReferenceEquals(route, CurationDeskWrites.Ingest)) return "{" + forged + IngestBody + "}"; + if (ReferenceEquals(route, CurationDeskWrites.RosterSet)) + return "{" + forged + "\"curator\":\"bob\",\"role\":\"curator\"}"; + if (ReferenceEquals(route, CurationDeskWrites.RosterRetire)) + return "{" + forged + "\"curator\":\"bob\"}"; return "{" + forged + "\"limit\":5}"; } @@ -538,6 +543,75 @@ public void ANameOutsideTheGrammarHasNoRecommenderPath(string username) Assert.Null(PrivateApi.CurationDeskRecommenderPath(username)); } + // ---- roster writes ------------------------------------------------------- + + [Fact] + public void ARosterSetForwardsOnlyTheCuratorFieldsAndNeverTheCallersIdentity() + { + var payload = Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"mod\",\"rules\":{\"trail\":false,\"min_weight\":1090}," + + "\"note\":\"mod only\",\"active\":true,\"added_by\":\"someone\",\"removed_at\":null}"); + Assert.Equal(new[] { "username", "curator", "role", "rules", "note" }, payload.Select(kv => kv.Key).ToArray()); + Assert.Equal("alice", payload["username"]!.GetValue()); + Assert.Equal("bob", payload["curator"]!.GetValue()); + // The rules object travels as sent: the backend stores it whole, and a key + // dropped here would silently change what the admin asked for. + var rules = (JsonObject)payload["rules"]!; + Assert.False(rules["trail"]!.GetValue()); + Assert.Equal(1090, rules["min_weight"]!.GetValue()); + } + + [Theory] + [InlineData("{\"role\":\"curator\"}", "curator required")] + [InlineData("{\"curator\":\"Bob\",\"role\":\"curator\"}", "curator required")] + [InlineData("{\"curator\":\"bob\\n\",\"role\":\"curator\"}", "curator required")] + [InlineData("{\"curator\":\"bob\"}", "invalid role")] + [InlineData("{\"curator\":\"bob\",\"role\":\"owner\"}", "invalid role")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":\"trail\"}", "rules must be an object")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"weight\":1}}", "unknown rule: weight")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"trail\":\"yes\"}}", "trail must be true or false")] + public void AMalformedRosterSetIsRefusedRatherThanTrimmed(string json, string expected) + { + Assert.Equal(expected, Rejected(CurationDeskWrites.RosterSet, json)); + } + + [Theory] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"min_weight\":10001}}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"min_weight\":-1}}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"min_weight\":true}}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"max_weight\":\"2000\"}}")] + [InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":{\"waves_only_below\":1.5}}")] + public void AWeightRuleOutsideTheVoteRangeIsRefused(string json) + { + Assert.Contains("vote weight between 0 and 10000", Rejected(CurationDeskWrites.RosterSet, json)); + } + + [Fact] + public void ACuratorNoteLongerThanTheColumnIsRefused() + { + var note = new string('x', 201); + Assert.Equal("invalid note", Rejected(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"note\":\"" + note + "\"}")); + Assert.True(Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"note\":\"" + note[..200] + "\"}").ContainsKey("note")); + } + + [Fact] + public void ARetireCarriesNothingButTheCurator() + { + var payload = Ok(CurationDeskWrites.RosterRetire, "{\"curator\":\"bob\",\"role\":\"admin\",\"force\":true}"); + Assert.Equal(new[] { "username", "curator" }, payload.Select(kv => kv.Key).ToArray()); + Assert.Equal("curator required", Rejected(CurationDeskWrites.RosterRetire, "{}")); + Assert.Equal("curator required", Rejected(CurationDeskWrites.RosterRetire, "{\"curator\":\"..\"}")); + } + + [Fact] + public void TheRosterListCarriesNothingTheCallerSent() + { + var payload = Ok(CurationDeskWrites.RosterList, "{\"limit\":5,\"role\":\"admin\",\"include_retired\":true}"); + Assert.Equal(new[] { "username" }, payload.Select(kv => kv.Key).ToArray()); + } + [Fact] public void TheRecommenderNameLengthBoundIsEnforced() { diff --git a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs index 1f677d2e..4bd8013d 100644 --- a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs +++ b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs @@ -430,6 +430,18 @@ public static Task CurationDeskMarks(HttpContext ctx) => public static Task CurationDeskCursor(HttpContext ctx) => ServeDeskWrite(ctx, CurationDeskWrites.Cursor); + // POST /private-api/curation-desk/roster-list + public static Task CurationDeskRosterList(HttpContext ctx) => + ServeDeskWrite(ctx, CurationDeskWrites.RosterList); + + // POST /private-api/curation-desk/roster-set + public static Task CurationDeskRosterSet(HttpContext ctx) => + ServeDeskWrite(ctx, CurationDeskWrites.RosterSet); + + // POST /private-api/curation-desk/roster-retire + public static Task CurationDeskRosterRetire(HttpContext ctx) => + ServeDeskWrite(ctx, CurationDeskWrites.RosterRetire); + // POST /private-api/curation-desk/recommend-meta public static Task CurationDeskRecommendMeta(HttpContext ctx) => ServeDeskWrite(ctx, CurationDeskWrites.RecommendMeta); @@ -749,6 +761,13 @@ public sealed record Route(string UpstreamPath, string[] Keys, bool ForwardClien public static readonly IReadOnlySet DismissActions = new HashSet { "dismiss", "restore" }; public static readonly IReadOnlySet UaClasses = new HashSet { "web", "mobile" }; public static readonly IReadOnlySet RosterSorts = new HashSet { "queue", "newest", "unique", "random" }; + /// The roles the backend's CHECK constraint accepts (spec 5.7). + public static readonly IReadOnlySet Roles = new HashSet { "admin", "mod", "curator", "trial" }; + /// The per-curator trailing rules, as Hive vote weights (100 = 1%). + public static readonly string[] RuleWeightKeys = { "min_weight", "max_weight", "waves_only_below" }; + public const int MaxVoteWeight = 10000; + /// The backend keeps a curator note in a varchar(200). + public const int MaxCuratorNoteLength = 200; /// The four event types erobot pushes (spec 7.2). public static readonly IReadOnlySet IngestTypes = new HashSet { "post", "vote", "curator_vote", "flag" }; /// The backend keeps the event id in a varchar(200). @@ -800,6 +819,20 @@ public sealed record Route(string UpstreamPath, string[] Keys, bool ForwardClien public static readonly Route Marks = new("curation/desk/marks/list", new[] { "state", "cursor", "limit" }); + /// + /// The roster write routes, all three admin-only upstream. They are POSTs rather + /// than an extension of the cached GET because the private view carries notes and + /// retired rows, and because a write must never be edge-cacheable. `rules` is the + /// one object here: unlike a read filter, an unknown key in it is refused rather + /// than dropped, so an admin is never told a rule was saved when it was discarded. + /// + public static readonly Route RosterList = new("curation/desk/roster/list", Array.Empty()); + + public static readonly Route RosterSet = new("curation/desk/roster/set", + new[] { "curator", "role", "rules", "note" }); + + public static readonly Route RosterRetire = new("curation/desk/roster/retire", new[] { "curator" }); + public static readonly Route Cursor = new("curation/desk/cursors", new[] { "post_id", "action", "reason" }); public static readonly Route RecommendMeta = new("curation/desk/recommendations/meta", @@ -1052,6 +1085,56 @@ private static void Truncate(JsonObject payload, string key, int max) { return RequireAuthorPermlink(body) ?? RequireOneOf(body, "action", DismissActions); } + if (ReferenceEquals(route, RosterSet)) + { + if (body.Str("curator") is not { } curator || !HiveNames.IsAccountName(curator)) + { + return "curator required"; + } + var roleError = RequireOneOf(body, "role", Roles); + if (roleError != null) return roleError; + if (body.TryGetPropertyValue("note", out var note) && note is not null + && body.Str("note") is not { Length: <= MaxCuratorNoteLength }) + { + return "invalid note"; + } + if (body.TryGetPropertyValue("rules", out var rules) && rules is not null) + { + if (rules is not JsonObject ruleObject) + { + return "rules must be an object"; + } + foreach (var rule in ruleObject) + { + if (rule.Key == "trail") + { + if (rule.Value?.GetValueKind() is not (JsonValueKind.True or JsonValueKind.False)) + { + return "trail must be true or false"; + } + continue; + } + if (Array.IndexOf(RuleWeightKeys, rule.Key) < 0) + { + return $"unknown rule: {rule.Key}"; + } + if (rule.Value is not JsonValue weightValue + || weightValue.GetValueKind() is not JsonValueKind.Number + || !weightValue.TryGetValue(out var weight) + || weight < 0 || weight > MaxVoteWeight) + { + return $"{rule.Key} must be a vote weight between 0 and {MaxVoteWeight}"; + } + } + } + return null; + } + if (ReferenceEquals(route, RosterRetire)) + { + return body.Str("curator") is { } name && HiveNames.IsAccountName(name) + ? null + : "curator required"; + } if (ReferenceEquals(route, Ingest)) { // The envelope shape the backend accepts (spec 7.2): anything else is a diff --git a/dotnet/EcencyApi/Handlers/Routes.cs b/dotnet/EcencyApi/Handlers/Routes.cs index 1c197950..46522d9f 100644 --- a/dotnet/EcencyApi/Handlers/Routes.cs +++ b/dotnet/EcencyApi/Handlers/Routes.cs @@ -188,6 +188,9 @@ public static void Map(WebApplication app) app.MapPost("/private-api/curation-desk/mark-clear", PrivateApi.CurationDeskMarkClear); app.MapPost("/private-api/curation-desk/marks", PrivateApi.CurationDeskMarks); app.MapPost("/private-api/curation-desk/cursor", PrivateApi.CurationDeskCursor); + app.MapPost("/private-api/curation-desk/roster-list", PrivateApi.CurationDeskRosterList); + app.MapPost("/private-api/curation-desk/roster-set", PrivateApi.CurationDeskRosterSet); + app.MapPost("/private-api/curation-desk/roster-retire", PrivateApi.CurationDeskRosterRetire); app.MapPost("/private-api/curation-desk/recommend-meta", PrivateApi.CurationDeskRecommendMeta); app.MapPost("/private-api/curation-desk/recommendation-dismiss", PrivateApi.CurationDeskRecommendationDismiss); app.MapPost("/private-api/curation-desk/ingest", PrivateApi.CurationDeskIngest); From d076fcf7d21e0f1078de0bf203f92d651894586b Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 10 Sep 2026 08:57:22 +0000 Subject: [PATCH 2/2] fix(curation desk): address the roster review findings Three from the bot review, all verified against the code first. The roster writes were added to the payload tests but not to SignedWrites(), the enumeration five shared tests iterate: auth, fail-closed, token forwarding, client-address isolation and cache-control. They behaved correctly, but nothing proved it. Marking roster-retire as forwarding the client address now fails two tests; it failed none before. A note was measured with string.Length, which counts UTF-16 code units, while the column is varchar(200) and the backend counts code points. 200 emoji measure 400 here, so the fence refused notes the column accepts. Counted in runes now, with a test at exactly 200 emoji and at 201. A present "rules": null skipped validation and CopyIfPresent forwarded the null through the allowlist, while the fence claimed every rules value is an object. Any present rules must now be one; absent is still absent, which is how an admin clears every rule. --- .../CurationDeskPayloadTests.cs | 26 +++++++++++++++++++ .../CurationDeskTestSupport.cs | 4 +++ .../Handlers/PrivateApi.CurationDesk.cs | 10 +++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs index 677635f0..a3d82815 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs @@ -596,6 +596,32 @@ public void ACuratorNoteLongerThanTheColumnIsRefused() "{\"curator\":\"bob\",\"role\":\"curator\",\"note\":\"" + note[..200] + "\"}").ContainsKey("note")); } + [Fact] + public void ANoteIsMeasuredTheWayTheColumnMeasuresIt() + { + // varchar(200) counts characters and Python's len() counts code points, but + // string.Length counts UTF-16 code units, so 200 emoji measure 400 and a note the + // column would have accepted was refused here. + var emoji = string.Concat(Enumerable.Repeat("\U0001F600", 200)); + Assert.Equal(400, emoji.Length); + Assert.True(Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"note\":\"" + emoji + "\"}").ContainsKey("note")); + Assert.Equal("invalid note", Rejected(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"note\":\"" + emoji + "\U0001F600\"}")); + } + + [Fact] + public void APresentButNullRulesIsRefusedRatherThanForwarded() + { + // CopyIfPresent forwards a null through the allowlist, so "rules": null would + // travel upstream while the fence claimed every rules value is an object. + Assert.Equal("rules must be an object", Rejected(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\",\"rules\":null}")); + // absent is still absent: that is how an admin clears every rule + Assert.False(Ok(CurationDeskWrites.RosterSet, + "{\"curator\":\"bob\",\"role\":\"curator\"}").ContainsKey("rules")); + } + [Fact] public void ARetireCarriesNothingButTheCurator() { diff --git a/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs b/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs index ccd0b37e..156c9135 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs @@ -258,5 +258,9 @@ public static TestClock UseTestClock() yield return ("recommendation-dismiss", PrivateApi.CurationDeskRecommendationDismiss, "{" + code + ",\"author\":\"bob\",\"permlink\":\"p\",\"action\":\"dismiss\"}"); yield return ("ingest", PrivateApi.CurationDeskIngest, "{" + code + ",\"v\":1,\"type\":\"flag\",\"id\":\"flag:bob/p:hivewatchers\",\"ts\":\"2026-09-05T10:00:00Z\",\"attempts\":0,\"payload\":{\"author\":\"bob\",\"permlink\":\"p\",\"weight\":-10000}}"); + yield return ("roster-list", PrivateApi.CurationDeskRosterList, "{" + code + "}"); + yield return ("roster-set", PrivateApi.CurationDeskRosterSet, + "{" + code + ",\"curator\":\"bob\",\"role\":\"curator\"}"); + yield return ("roster-retire", PrivateApi.CurationDeskRosterRetire, "{" + code + ",\"curator\":\"bob\"}"); } } diff --git a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs index 4bd8013d..800e7e24 100644 --- a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs +++ b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs @@ -1093,12 +1093,18 @@ private static void Truncate(JsonObject payload, string key, int max) } var roleError = RequireOneOf(body, "role", Roles); if (roleError != null) return roleError; + // varchar(200) counts CHARACTERS and Python's len() counts code points, but + // string.Length counts UTF-16 code units: 200 emoji measure 400 here and would + // be refused by the fence though the column accepts them. Count runes instead. if (body.TryGetPropertyValue("note", out var note) && note is not null - && body.Str("note") is not { Length: <= MaxCuratorNoteLength }) + && (body.Str("note") is not { } text + || text.EnumerateRunes().Count() > MaxCuratorNoteLength)) { return "invalid note"; } - if (body.TryGetPropertyValue("rules", out var rules) && rules is not null) + // A PRESENT `rules` must be an object, null included. `CopyIfPresent` forwards + // a null through the allowlist, and the fence should say what the contract says. + if (body.TryGetPropertyValue("rules", out var rules)) { if (rules is not JsonObject ruleObject) {