Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/community/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,22 @@ export const ChallengeOptionInputSchema = z.looseObject({
required: z.boolean().optional() // If this is true, the challenge option is required, the challenge will throw without it
}); // should be flexible

export const ChallengeResultSchema = z.object({ success: z.literal(true) }).or(z.object({ success: z.literal(false), error: z.string() }));
// `comment` and `commentUpdate` are typed as loose records here — allowed-key validation happens in
// validateChallengeResultExtras during aggregation, so we can emit a precise PKCError with
// {challengeIndex, offendingKey} instead of a generic Zod failure.
export const ChallengeResultSchema = z
.object({
success: z.literal(true),
comment: z.looseObject({}).optional(),
commentUpdate: z.looseObject({}).optional()
})
.or(
z.object({
success: z.literal(false),
error: z.string(),
reason: z.string().optional()
})
);

export const ChallengeFromGetChallengeSchema = z
.object({
Expand Down
6 changes: 5 additions & 1 deletion src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,5 +364,9 @@ export enum messages {
ERR_RPC_CLIENT_TRYING_TO_DELETE_REMOTE_COMMUNITY = "RPC client is attempting to delete remote community",
ERR_GENERIC_RPC_CLIENT_CALL_ERROR = "RPC client received an unknown error when executing call over websocket",
ERR_RPC_CLIENT_CHALLENGE_NAME_NOT_AVAILABLE_ON_SERVER = "The challenge name is not available on the RPC server. Available challenges are listed in details.availableChallenges",
ERR_ROLE_ADDRESS_NAME_COULD_NOT_BE_RESOLVED = "Role address name could not be resolved"
ERR_ROLE_ADDRESS_NAME_COULD_NOT_BE_RESOLVED = "Role address name could not be resolved",
ERR_CHALLENGE_RESULT_OVERRIDES_COMMENT_SIGNED_FIELD = "A challenge result attempted to set comment.<field> that is in CommentSignedPropertyNames (would invalidate the author's signature on CommentIpfs)",
ERR_CHALLENGE_RESULT_OVERRIDES_RESERVED_COMMENT_UPDATE_FIELD = "A challenge result attempted to set commentUpdate.<field> that is in CommentUpdateChallengeReservedFieldNames (community-computed field)",
ERR_CHALLENGE_RESULT_OVERRIDES_NON_COMMUNITY_AUTHOR_KEY = "A challenge result attempted to set commentUpdate.author.<field> with a key other than 'community' — challenges may only extend commentUpdate.author.community",
ERR_CHALLENGE_RESULT_OVERRIDES_RESERVED_COMMUNITY_AUTHOR_FIELD = "A challenge result attempted to set commentUpdate.author.community.<field> that is in CommunityAuthorChallengeReservedFieldNames (community-computed or mod-settable field)"
}
33 changes: 33 additions & 0 deletions src/publications/comment/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,32 @@ export const CommentUpdateSchema = z

export const CommentUpdateSignedPropertyNames = remeda.keys.strict(remeda.omit(CommentUpdateSchema.shape, ["signature"]));

// Community-computed fields on CommentUpdate. Challenges may not set these via the `commentUpdate`
// field on a ChallengeResult; any other key (including new unknown ones invented by external
// challenges) is allowed and shallow-merged with lowest priority. `satisfies` ties each entry to a
// real CommentUpdate key so renaming a field on CommentUpdateSchema surfaces a stale entry at compile time.
// NOTE: `author` is allowed here but validated separately by validateChallengeResultExtras — only
// `author.community.<key>` is permitted. Non-`community` author keys are rejected, and schema-defined
// `author.community` keys are rejected EXCEPT the ones intentionally left out of
// CommunityAuthorChallengeReservedFieldNames (currently `flairs`, which a challenge may seed as
// lowest priority and a mod's commentModeration.author.flairs overrides).
export const CommentUpdateChallengeReservedFieldNames = [
"signature",
"cid",
"upvoteCount",
"downvoteCount",
"replyCount",
"childCount",
"number",
"postNumber",
"updatedAt",
"lastChildCid",
"lastReplyTimestamp",
"replies",
"edit",
"protocolVersion"
] as const satisfies readonly (keyof z.infer<typeof CommentUpdateSchema>)[];

export const CommentUpdateForDisapprovedPendingComment = CommentUpdateSchema.pick({
author: true,
cid: true,
Expand All @@ -174,6 +200,10 @@ export const CommentUpdateForDisapprovedPendingCommentSignedPropertyNames = reme
remeda.omit(CommentUpdateForDisapprovedPendingComment.shape, ["signature"])
);

// Strict for declared fields; challenge-supplied unknown keys (e.g. `reason`, `countryCode`) are
// merged in at runtime by storePublicationAndEncryptForChallengeVerification and the dynamic
// signedPropertyNames is computed at sign time as the union of these picked fields plus the actual
// extra keys present on the merged object.
export const CommentUpdateForChallengeVerificationSchema = CommentUpdateSchema.pick({
author: true,
cid: true,
Expand All @@ -198,6 +228,9 @@ export const CommentsTableRowSchema = CommentIpfsSchema.extend({
authorSignerAddress: SignerWithAddressPublicKeySchema.shape.address,
originalCommentSignatureEncoded: CommentPubsubMessagePublicationSchema.shape.signature.shape.signature.optional(),
extraProps: z.looseObject({}).optional(),
// challenge-supplied partial CommentUpdate, shallow-merged across successful challenges, seeded
// into queryCalculatedCommentUpdate with lowest priority (per-field overridden by mod queries).
challengeCommentUpdate: z.looseObject({}).optional(),
pendingApproval: z.boolean().optional(),
number: z.number().int().positive().optional(),
postNumber: z.number().int().positive().optional()
Expand Down
71 changes: 63 additions & 8 deletions src/runtime/node/community/challenges/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,49 @@ import * as remeda from "remeda";
import { ChallengeFileFactorySchema, ChallengeFileSchema, CommunityChallengeSettingSchema } from "../../../../community/schema.js";
import { PKCError } from "../../../../pkc-error.js";
import { pathToFileURL } from "node:url";
import { validateChallengeResultExtras } from "./validate-challenge-result.js";

type PendingChallenge = Challenge & { index: number };

type DeferredChallenge = { index: number; communityChallenge: CommunityChallenge };

export type GetChallengeAnswers = (challenges: Omit<Challenge, "verify">[]) => Promise<DecryptedChallengeAnswer["challengeAnswers"]>;

type ChallengeVerificationSuccess = { challengeSuccess: true; pendingApprovalSuccess: boolean };
export type ChallengeResultAggregate = {
aggregatedComment?: Record<string, unknown>;
aggregatedCommentUpdate?: Record<string, unknown>;
aggregatedReason?: string;
};

// Mutates `agg` in place — merges challenge-supplied `comment`/`commentUpdate` extras from one
// successful result, or the `reason` from a failure. Throws via the guard if the result tries to
// override a protocol-owned field. Called for both immediate (Phase 5) and post-verify results.
const accumulateChallengeResultExtras = ({
challengeResult,
challengeIndex,
agg
}: {
challengeResult: ChallengeResult;
challengeIndex: number;
agg: ChallengeResultAggregate;
}): void => {
validateChallengeResultExtras({ challengeResult, challengeIndex });
if (!("success" in challengeResult) || challengeResult.success !== true) {
if (challengeResult.reason) agg.aggregatedReason = challengeResult.reason;
return;
}
if (challengeResult.comment) {
Comment on lines +62 to +65
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve empty-string reason values during aggregation.

Using truthy checks drops valid reason: "" values. Use explicit !== undefined checks so all schema-valid reasons propagate consistently.

Suggested fix
-    if (!("success" in challengeResult) || challengeResult.success !== true) {
-        if (challengeResult.reason) agg.aggregatedReason = challengeResult.reason;
+    if (!("success" in challengeResult) || challengeResult.success !== true) {
+        if (challengeResult.reason !== undefined) agg.aggregatedReason = challengeResult.reason;
         return;
     }
-    if (challengeResult.reason) agg.aggregatedReason = challengeResult.reason;
+    if (challengeResult.reason !== undefined) agg.aggregatedReason = challengeResult.reason;
-        if (res.aggregatedReason) challengeVerification.aggregatedReason = res.aggregatedReason;
+        if (res.aggregatedReason !== undefined) challengeVerification.aggregatedReason = res.aggregatedReason;

Also applies to: 807-810

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/node/community/challenges/index.ts` around lines 62 - 66, The
code currently uses truthy checks like `if (challengeResult.reason)` before
assigning to `agg.aggregatedReason`, which drops valid empty-string reasons;
change these checks to explicit undefined comparisons (`if
(challengeResult.reason !== undefined)`) wherever `challengeResult.reason` is
inspected (including the occurrences around the early return and the later
assignment and the similar block at lines ~807-810) so empty-string values are
preserved and still propagated to `agg.aggregatedReason`.

agg.aggregatedComment = { ...(agg.aggregatedComment ?? {}), ...challengeResult.comment };
}
if (challengeResult.commentUpdate) {
agg.aggregatedCommentUpdate = { ...(agg.aggregatedCommentUpdate ?? {}), ...challengeResult.commentUpdate };
}
};

type ChallengeVerificationSuccess = { challengeSuccess: true; pendingApprovalSuccess: boolean } & Omit<
ChallengeResultAggregate,
"aggregatedReason"
>;
type ChallengeVerificationPending = {
pendingChallenges: PendingChallenge[];
pendingApprovalSuccess: boolean;
Expand All @@ -49,7 +84,7 @@ type ChallengeVerificationPending = {
type ChallengeVerificationFailure = {
challengeSuccess: false;
challengeErrors: NonNullable<ChallengeVerificationMessageType["challengeErrors"]>;
};
} & Pick<ChallengeResultAggregate, "aggregatedReason">;

// Use structural typing for the pkc param to avoid circular import issues
type PKCWithSettingsChallenges = {
Expand Down Expand Up @@ -487,15 +522,18 @@ const getPendingChallengesOrChallengeVerification = async ({
let pendingChallenges: PendingChallenge[] = [];
const challengeErrors: NonNullable<ChallengeVerificationMessageType["challengeErrors"]> = {};
let pendingApprovalSuccess = false;
const agg: ChallengeResultAggregate = {};
for (let i = 0; i < challengeCount; i++) {
const r = results[i];
if (r === undefined) continue; // excluded earlier (phase 1 or phase 2)
if (shouldExcludeChallengeSuccess(communityChallenges[i], i, results as (Challenge | ChallengeResult)[])) continue;
if ("success" in r && r.success === false) {
challengeFailureCount++;
challengeErrors[i] = r.error;
accumulateChallengeResultExtras({ challengeResult: r, challengeIndex: i, agg });
} else if ("success" in r && r.success === true) {
if (communityChallenges[i].pendingApproval) pendingApprovalSuccess = true;
accumulateChallengeResultExtras({ challengeResult: r, challengeIndex: i, agg });
} else {
pendingChallenges.push({ ...r, index: i });
}
Expand All @@ -512,8 +550,11 @@ const getPendingChallengesOrChallengeVerification = async ({
challengeSuccess = true;
}

if (challengeSuccess === true) return { challengeSuccess, pendingApprovalSuccess };
if (challengeSuccess === false) return { challengeSuccess, challengeErrors };
if (challengeSuccess === true) {
const { aggregatedReason: _unusedOnSuccess, ...successAgg } = agg;
return { challengeSuccess, pendingApprovalSuccess, ...successAgg };
}
if (challengeSuccess === false) return { challengeSuccess, challengeErrors, aggregatedReason: agg.aggregatedReason };
return {
pendingChallenges,
pendingApprovalSuccess,
Expand Down Expand Up @@ -685,32 +726,42 @@ const getChallengeVerificationFromChallengeAnswers = async ({
let challengeFailureCount = 0;
const challengeErrors: NonNullable<ChallengeVerificationMessageType["challengeErrors"]> = {};
let pendingApprovalSuccess = false;
const agg: ChallengeResultAggregate = {};
for (let i = 0; i < challengeCount; i++) {
const r = results[i];
if (r === undefined) continue;
if ("success" in r && r.success === false) {
if (shouldExcludeChallengeSuccess(loadedCommunityChallenges[i], i, results as (Challenge | ChallengeResult)[])) continue;
challengeFailureCount++;
challengeErrors[i] = r.error;
accumulateChallengeResultExtras({ challengeResult: r, challengeIndex: i, agg });
} else if ("success" in r && r.success === true) {
if (shouldExcludeChallengeSuccess(loadedCommunityChallenges[i], i, results as (Challenge | ChallengeResult)[])) continue;
if (loadedCommunityChallenges[i].pendingApproval) pendingApprovalSuccess = true;
accumulateChallengeResultExtras({ challengeResult: r, challengeIndex: i, agg });
}
// Any pending shape left at this point would be unexpected; ignore.
}

if (challengeFailureCount > 0) {
return {
challengeSuccess: false,
challengeErrors
challengeErrors,
aggregatedReason: agg.aggregatedReason
};
}
const { aggregatedReason: _unusedOnSuccess, ...successAgg } = agg;
return {
challengeSuccess: true,
pendingApprovalSuccess
pendingApprovalSuccess,
...successAgg
};
};

export type GetChallengeVerificationResult = Pick<ChallengeVerificationMessageType, "challengeErrors" | "challengeSuccess"> & {
pendingApproval?: boolean;
} & ChallengeResultAggregate;

const getChallengeVerification = async ({
challengeRequestMessage,
community,
Expand All @@ -719,7 +770,7 @@ const getChallengeVerification = async ({
challengeRequestMessage: DecryptedChallengeRequestMessageTypeWithCommunityAuthor;
community: LocalCommunity;
getChallengeAnswers: GetChallengeAnswers;
}): Promise<Pick<ChallengeVerificationMessageType, "challengeErrors" | "challengeSuccess"> & { pendingApproval?: boolean }> => {
}): Promise<GetChallengeVerificationResult> => {
if (!challengeRequestMessage) {
throw Error(`getChallengeVerification invalid challengeRequestMessage argument '${challengeRequestMessage}'`);
}
Expand All @@ -734,7 +785,7 @@ const getChallengeVerification = async ({
const res = await getPendingChallengesOrChallengeVerification({ challengeRequestMessage, community });
let pendingApprovalSuccess = "pendingApprovalSuccess" in res ? res.pendingApprovalSuccess : false;

let challengeVerification: Pick<ChallengeVerificationMessageType, "challengeSuccess" | "challengeErrors">;
let challengeVerification: Pick<ChallengeVerificationMessageType, "challengeSuccess" | "challengeErrors"> & ChallengeResultAggregate;
// was able to verify without asking author for challenges
if ("pendingChallenges" in res) {
const challengeAnswers = await getChallengeAnswers(
Expand All @@ -759,6 +810,10 @@ const getChallengeVerification = async ({
} else {
challengeVerification = { challengeSuccess: res.challengeSuccess };
if ("challengeErrors" in res) challengeVerification.challengeErrors = res.challengeErrors;
if ("aggregatedComment" in res && res.aggregatedComment) challengeVerification.aggregatedComment = res.aggregatedComment;
if ("aggregatedCommentUpdate" in res && res.aggregatedCommentUpdate)
challengeVerification.aggregatedCommentUpdate = res.aggregatedCommentUpdate;
if ("aggregatedReason" in res && res.aggregatedReason) challengeVerification.aggregatedReason = res.aggregatedReason;
}

// store the publication result and author address in mem cache for rateLimit exclude challenge settings
Expand Down
76 changes: 76 additions & 0 deletions src/runtime/node/community/challenges/validate-challenge-result.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { CommentSignedPropertyNames, CommentUpdateChallengeReservedFieldNames } from "../../../../publications/comment/schema.js";
import { CommunityAuthorChallengeReservedFieldNames } from "../../../../schema/schema.js";
import { PKCError } from "../../../../pkc-error.js";
import type { ChallengeResult } from "../../../../community/types.js";

// Throws when a challenge result tries to set a key that the protocol owns.
// - comment.<key>: forbidden if key is in CommentSignedPropertyNames (would invalidate the author's
// signature on CommentIpfs, since challenge fields are spread last into the IPFS-bound literal).
// - commentUpdate.<key>: forbidden if key is in CommentUpdateChallengeReservedFieldNames
// (community-computed fields: counts, cid, signature, updatedAt, etc.).
// - commentUpdate.author.<key>: only `community` is permitted (rest of author is signed identity).
// - commentUpdate.author.community.<key>: forbidden if key is in CommunityAuthorChallengeReservedFieldNames
// (computed scores/timestamps and mod-settable fields; mods own those via commentModeration).
export function validateChallengeResultExtras({
challengeResult,
challengeIndex
}: {
challengeResult: ChallengeResult;
challengeIndex: number;
}): void {
if (!("success" in challengeResult) || challengeResult.success !== true) return;
if (challengeResult.comment) {
for (const key of Object.keys(challengeResult.comment)) {
if ((CommentSignedPropertyNames as readonly string[]).includes(key)) {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_COMMENT_SIGNED_FIELD", {
challengeIndex,
offendingKey: key
});
}
}
}
if (challengeResult.commentUpdate) {
for (const key of Object.keys(challengeResult.commentUpdate)) {
if ((CommentUpdateChallengeReservedFieldNames as readonly string[]).includes(key)) {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_RESERVED_COMMENT_UPDATE_FIELD", {
challengeIndex,
offendingKey: key
});
}
}
const author = (challengeResult.commentUpdate as { author?: unknown }).author;
if (author !== undefined) {
if (typeof author !== "object" || author === null || Array.isArray(author)) {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_NON_COMMUNITY_AUTHOR_KEY", {
challengeIndex,
offendingKey: "author"
});
}
for (const authorKey of Object.keys(author as Record<string, unknown>)) {
if (authorKey !== "community") {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_NON_COMMUNITY_AUTHOR_KEY", {
challengeIndex,
offendingKey: `author.${authorKey}`
});
}
}
const community = (author as { community?: unknown }).community;
if (community !== undefined) {
if (typeof community !== "object" || community === null || Array.isArray(community)) {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_RESERVED_COMMUNITY_AUTHOR_FIELD", {
challengeIndex,
offendingKey: "author.community"
});
}
for (const communityKey of Object.keys(community as Record<string, unknown>)) {
if ((CommunityAuthorChallengeReservedFieldNames as readonly string[]).includes(communityKey)) {
throw new PKCError("ERR_CHALLENGE_RESULT_OVERRIDES_RESERVED_COMMUNITY_AUTHOR_FIELD", {
challengeIndex,
offendingKey: `author.community.${communityKey}`
});
}
}
}
}
}
}
Loading
Loading