diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d342d8..34bec4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ The format is based on [Keep a Changelog 1.1.0](https://keepachangelog.com/en/1. ## [Unreleased] +### Security: App Review mod-permission gate (2026-06-07) + +Reddit App Review rejected cm-devvit 0.3.0 on mod-permission handling. This wave closes the gap: every mod-only surface is verified server-side, and the permission model is documented for installing subreddits. + +- **Gate all mod-menu handlers with `requireModerator()`** (`src/routes/menu.ts`): the six `/internal/menu/*` handlers (reload-config, recent-actions, set-openai-key, explain-rule, simulate-rule, test-rules) previously relied only on the menu item's `forUserType: "moderator"`. But the handler endpoints are HTTP-reachable by any viewer of the Observatory custom post (the same reachability the `/api/*` and form-submit handlers already defend against), so a non-mod could POST them directly. They now verify moderator status server-side before reading the wiki, publishing config, or creating the dashboard post, and return a "Mod-only action" toast otherwise. The two action handlers resolve the sub via `requireModerator()` (dropping a redundant `getCurrentSubreddit` call). Pinned by `tests/routes/menu-auth.test.ts`. +- **Extract shared `authFailToast` helper** (`src/lib/authFailToast.ts`): the toast wording (including the Polish #10 transient-vs-terminal split) was a local function in `forms.ts`. Extracted so `menu.ts` reuses the exact same copy. One source of truth, same anti-drift rationale as `openaiErrors.ts`. Pinned by `tests/lib/authFailToast.test.ts`. +- **Client "Moderators only" gate** (`src/client/{App.tsx,lib/api.ts,lib/types.ts}`): when a dashboard data endpoint replies `403`, the webview shows a dedicated "Moderators only" notice instead of the misleading "Telemetry API unreachable" retry banner. The gate triggers strictly on `403` (a `forbidden` flag on the API result), so a transient `503`/`500` for a real mod still shows the retry banner. Pinned by `tests/client/app.test.tsx` + `tests/client/api.test.ts`. +- **Document the permission model** (`README.md`): new "Moderator permissions and data access" section enumerates every gated surface, states the gate is "is a moderator of this subreddit" (binary, not granular per Reddit mod-permission), and documents the intentional exception (any moderator can edit config via the app, analogous to AutoModerator) plus the safe unauthenticated exceptions (`/api/health` liveness, `?demo=1` synthetic fixtures, platform-only trigger/cron routes). The FAQ "Can other mods edit the config?" answer is corrected to describe both the in-app editor path and the direct-wiki path. + +Verified: `npm run type-check` and `npm run lint` clean, 923/923 tests pass. A second-model adversarial audit (gemini-agent, since the Codex CLI was unavailable) confirmed every mod-data and mod-action endpoint is gated and fails closed. + Forward-looking (post-v0.6.7): see [`ROADMAP.md`](./ROADMAP.md). ## [0.6.7]: 2026-05-19 diff --git a/README.md b/README.md index 7831cca..d132377 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,37 @@ The Devvit port preserves the rule/check/action concept model that 15+ existing > **No hosting. No tokens. No central bottleneck.** Everything lives inside your subreddit's Devvit installation. +## Moderator permissions and data access + +ContextMod is a moderator-only tool. Every surface that shows moderation data or performs a moderation action is gated on the caller being a moderator of the subreddit, verified server-side against Reddit's live moderator list (`reddit.getModerators()`) on every request, not just by the menu's `forUserType`. This section is the "who can see and do what" contract for any subreddit installing the app. + +**Menu items.** All six mod-menu entries (Reload config, View recent actions, Test rules, Simulate rule, Explain rule with AI, Set OpenAI API key) declare `"forUserType": "moderator"` in `devvit.json`, so non-moderators never see them in the menu. + +**Server-side verification (defense in depth).** The Observatory custom post is a webview, and its server endpoints are HTTP-reachable by anyone who can open the post, not only by the mod who clicked a menu item. So `forUserType` alone is not a security boundary. Every endpoint that returns mod data or performs an action calls `requireModerator()` before doing any work. The dashboard data and action API replies `403`; the menu and form handlers reply with a "Mod-only action" toast (the Devvit response shape they require). In both cases the work never runs for a non-moderator. + +| Surface | Endpoint(s) | Gate | +|---|---|---| +| Recent actions feed | `GET /api/recent` | `requireModerator` | +| Stats counters | `GET /api/stats` | `requireModerator` | +| Mod-activity feed | `GET /api/mod-activity` | `requireModerator` | +| Config revision history | `GET /api/config-history` | `requireModerator` | +| Muted rules (read + mutate) | `GET /api/muted-rules`, `POST /api/mute-rule`, `POST /api/unmute-rule` | `requireModerator` | +| AI event explainer | `POST /api/explain-event` | `requireModerator` | +| Config editor | `GET /api/config/raw`, `POST /api/config/{validate,simulate-live,explain,save}` | `requireModerator` | +| Mod menu actions + forms | `POST /internal/menu/*`, `POST /internal/form/*` | `requireModerator` | + +A non-moderator who opens the Observatory post sees a "Moderators only" notice, never moderation data. + +**Permission granularity (documented exception).** The gate is "is a moderator of this subreddit." ContextMod does not distinguish granular Reddit moderator permissions (`wiki`, `config`, `posts`, `flair`, and so on): any moderator can view the telemetry and edit the rule config through the app. This is intentional. ContextMod's wiki config is the bot's control surface, managed by the moderator team as a whole, the same way AutoModerator's config is. The in-app editor writes the config wiki page on behalf of any moderator using the app's moderator scope. If your team needs to restrict who changes the bot's behavior, use Reddit's native moderator permissions plus your team's own process. + +**Exceptions to the mod gate (and why they are safe).** + +- `GET /api/health` is an unauthenticated liveness probe. It returns only `{ok, app name, version, timestamp}`: no moderation data, no Redis access. The dashboard's reload button and external uptime checks use it. `GET /api/health/deep` (which does touch Redis and Reddit context) IS mod-gated. +- `?demo=1` on the read endpoints returns clearly-synthetic fixtures (`demo_mod_alice`, and so on) so the dashboard can be demoed without a live install. Real subreddit data is never served on the demo path. +- `/internal/triggers/*` and `/internal/cron/*` are invoked by the Devvit platform (event triggers and schedulers), not by users, so they are not part of the moderator-facing surface. + +**Data isolation.** Each install runs in its own Devvit Redis namespace, so one subreddit's moderation data is never visible to another. + ## Try locally in 3 commands (for judges + devs) See the Observatory dashboard render without installing the app, no Devvit credentials needed: @@ -408,7 +439,7 @@ The concept model, schema validation, config publish pipeline, idempotency primi No. Devvit runs the server. You install via the Reddit App Directory, write your rules in your sub's wiki, and that's it. **Can other mods edit the config?** -Yes, anyone with `wiki` permissions in your sub can edit `/wiki/botconfig/contextmod`. Standard Reddit wiki access control applies. +Two paths. (1) Through the app: any moderator can edit and save the config via the in-app editor. The app writes the wiki page using its own moderator scope, so a granular `wiki` permission is not required in-app (see [Moderator permissions and data access](#moderator-permissions-and-data-access) for why this is intentional). (2) Directly: editing `/wiki/botconfig/contextmod` outside the app follows standard Reddit wiki access control, which needs the `wiki` moderator permission. **What happens if I edit the wiki and break the config?** The 5-minute refresh cron validates new config against an AJV JSON Schema. If it fails to parse or validate, the previous `cfg:current_rev` stays active and the error is logged. Your sub stays moderated by the last good config until you fix the wiki page. diff --git a/docs/submission/app-review-feedback-2026-06-07.md b/docs/submission/app-review-feedback-2026-06-07.md new file mode 100644 index 0000000..45b8db7 --- /dev/null +++ b/docs/submission/app-review-feedback-2026-06-07.md @@ -0,0 +1,63 @@ +# Reddit App Review feedback response: mod permissions (2026-06-07) + +Structured response to the Reddit App Review rejection of cm-devvit 0.3.0, received 2026-06-07 in the r/Devvit App Review chat thread. 0.3.0 was the first version the reviewer could actually test: 0.2.7 auto-rejected on subreddit privacy before any functional review (the sub has been public since 2026-05-24). This wave closes the mod-permission gap, then re-submits via `npm run launch`. + +## Rejection (verbatim) + +From r/Devvit (App Review), 2026-06-07 1:00 PM, to u/CowSufficient3840: + +> Thank you for submitting cm-devvit (cm-devvit). Please ensure the following is resolved before an approval can be made. +> +> 1: Privacy and Data Rules: Be transparent – be up front about your data practices, ensure all consents and permissions are complete, accurate, and clearly labeled, and notify Reddit and your users if your app is compromised (e.g., data breach, unauthorized access). +> +> Your mod tool doesn't handle mod permissions correctly. Remember: +> +> - Any mod-only menu items should use "forUserType": "moderator" +> - Any mod-only forms or webview posts should only display mod data to mods with proper permissions to see that data +> - Any mod-only actions should check for proper mod perms before allowing the action to be submitted. +> +> Any exceptions must have a valid use case for your tool and be properly documented in your README so subreddits using the app know what to expect. + +## Audit findings (tiered) + +### TIER 1 (BLOCKER, fixed this wave) + +**Mod menu action handlers did not verify mod status server-side.** All six `/internal/menu/*` handlers relied only on the menu item's `forUserType: "moderator"`. Those handler endpoints are HTTP-reachable by any authenticated viewer of the Observatory custom post (the same class of reachability the `/api/*` and `/internal/form/*` handlers already defend against, per Polish #135 and Wave W). `reload-config` (publishes config to internal state) and `recent-actions` (creates a custom post) were the live exposures; the four form-display handlers were lower risk but are gated too for a uniform posture. This is the literal subject of the reviewer's bullet 3. + +Fix: `requireModerator()` on all six handlers (`src/routes/menu.ts`), shared `authFailToast` (`src/lib/authFailToast.ts`), regression test `tests/routes/menu-auth.test.ts`. + +### TIER 2 (HIGH, fixed this wave) + +**The non-mod webview state was a misleading error banner, not an explicit gate.** The `/api/*` data endpoints were already mod-gated (Polish #135 and Wave W), so a non-mod never received mod data. But the dashboard rendered an empty shell plus a red "Telemetry API unreachable" banner, which reads as a bug rather than an intentional permission boundary. Reviewer bullet 2 is about webview posts displaying mod data only to mods; an explicit gate removes the ambiguity. + +Fix: client "Moderators only" notice on `403` (`src/client/App.tsx`, `lib/api.ts`, `lib/types.ts`), tests in `tests/client/`. + +### TIER 3 (DOC, fixed this wave) + +**The permission model was not documented for installing subreddits.** Reviewer bullet 4 requires that any exceptions have a valid use case documented in the README. The gate is binary "is a moderator" (not granular per Reddit mod-permission), and the in-app editor lets any moderator edit config via the app's moderator scope. Both are intentional and now documented. + +Fix: README "Moderator permissions and data access" section + corrected FAQ. + +## Bullet-by-bullet status + +1. Mod-only menu items use `forUserType: "moderator"`: already true in `devvit.json` for all six items (no code change; now documented). +2. Forms/webview display mod data only to mods: server-gated already; the client gate now makes it explicit. +3. Mod-only actions check perms before submit: now gated server-side on all menu, form, and api handlers. +4. Exceptions documented in README: new permission-model section. + +## On granular permissions + +The reviewer wrote "proper mod perms." ContextMod gates on subreddit moderator membership (binary), not on granular Reddit moderator permissions (`wiki` / `config` / `flair` / etc). This is intentional and documented as the "exception" the reviewer's bullet 4 anticipates: ContextMod's wiki config is the bot's control surface, managed by the moderator team as a whole, the same way AutoModerator's config is. Restricting config editing to specific mods is left to Reddit's native moderator-permission system plus the team's own process. The binary model also matches the prior production smoke test (Polish #135) that proved `requireModerator` returns 403 to non-mods and 200 to mods in the live webview. + +## Verification + +- `npm run type-check`, `npm run lint`: clean. +- `npm run test`: 923/923 pass (adds menu-auth, authFailToast, and client 403-gate coverage). +- Adversarial review: the Codex CLI was unavailable (account model restriction), so a second-model audit was run via gemini-agent. It enumerated every route and confirmed every mod-data or mod-action endpoint calls `requireModerator()` and fails closed; the only ungated endpoints are the unauthenticated liveness probe (`/api/health`, no data), platform-fired triggers, and platform-fired crons. + +## Remaining (Stephen-side) + +- Confirm r/cm_devvit_test is still public and the Observatory example post is visible, so the reviewer can test (the sub-privacy gotcha that auto-rejected 0.2.7). +- After CI is green on main, re-submit via `npm run launch` (ships the next patch, 0.3.1, for review). +- Optional reply in the App Review chat once re-submitted (Stephen sends; the CLI cannot drive Reddit chat). +- Live smoke (nice-to-have): hit a `/api/*` route and a `/internal/menu/*` route from a non-mod test account to empirically confirm the 403 / mod-only toast in the menu execution context (the source is correct by inspection and the form-submit context was already live-verified). diff --git a/src/client/App.tsx b/src/client/App.tsx index 96e3013..7d061af 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -44,6 +44,9 @@ export default function App() { const [tourOpen, setTourOpen] = useState(() => !hasSeenTour()); const [historyOpen, setHistoryOpen] = useState(false); const [editorOpen, setEditorOpen] = useState(false); + // App Review (2026-06-07): when the API 403s (viewer is not a mod), show a + // dedicated "moderators only" notice instead of the retry banner. + const [forbidden, setForbidden] = useState(false); // Y2-X56: track initial load so first paint shows shimmer skeletons // instead of the empty-state CTA (which would mislead the mod into // thinking the bot is idle when actually we just haven't fetched yet). @@ -55,12 +58,24 @@ export default function App() { // If EITHER call errored, surface the error and keep the last-good state // so the dashboard doesn't lose its display while the API recovers. if (!recent.ok || !statsData.ok) { + // A 403 (not a moderator) is not an outage. The server already gates + // every data endpoint; render the dedicated "moderators only" notice + // rather than the "Telemetry API unreachable" retry banner so a non-mod + // viewing the Observatory post sees an accurate, intentional state. + if ((!recent.ok && recent.forbidden) || (!statsData.ok && statsData.forbidden)) { + setForbidden(true); + setInitialLoad(false); + setRefreshedAt(Date.now()); + return; + } const errMsg = !recent.ok ? recent.error : !statsData.ok ? statsData.error : 'unknown'; setApiError(errMsg); setRefreshedAt(Date.now()); return; } + // Recovered (or a mod was just added): clear any prior forbidden gate. + setForbidden(false); setApiError(null); // Both calls succeeded. Three branches: real data, empty + demo, empty + zero-state. @@ -138,6 +153,26 @@ export default function App() { ); useKeyboardShortcuts(shortcuts); + // Moderators-only gate (App Review 2026-06-07). All hooks above run + // unconditionally; this early return is safe below them. + if (forbidden) { + return ( +
+
+

+ Moderators only +

+

+ The ContextMod Observatory dashboard is visible only to moderators of + r/{subreddit}. Mod-action telemetry, rule config, and AI tools are + gated server-side. If you moderate this community, sign in with the + account that holds your mod permissions. +

+
+
+ ); + } + return (
{ export async function fetchRecentSafe(): Promise> { try { const res = await fetch(`/api/recent${demoSuffix()}`); - if (!res.ok) return { ok: false, error: await extractServerError(res) }; + // forbidden: a 403 means "not a moderator", not an outage — the dashboard + // renders a "moderators only" notice instead of the retry banner. + if (!res.ok) + return { ok: false, error: await extractServerError(res), forbidden: res.status === 403 }; const data = await res.json(); const events = Array.isArray(data?.events) ? (data.events as EventRecord[]) : []; if (events.length === 0) return { ok: true, empty: true }; @@ -57,7 +60,8 @@ export async function fetchRecentSafe(): Promise> { export async function fetchStatsSafe(): Promise> { try { const res = await fetch(`/api/stats${demoSuffix()}`); - if (!res.ok) return { ok: false, error: await extractServerError(res) }; + if (!res.ok) + return { ok: false, error: await extractServerError(res), forbidden: res.status === 403 }; const data = await res.json(); const c = data?.counters; if (!c || typeof c !== 'object' || !Array.isArray(c.hourlyActions24h)) { diff --git a/src/client/lib/types.ts b/src/client/lib/types.ts index a6ce677..4a0cb6f 100644 --- a/src/client/lib/types.ts +++ b/src/client/lib/types.ts @@ -57,13 +57,18 @@ export type StatsRollup = { * - { ok: true, empty: true }: server returned 200 + empty payload → show zero-state (fresh install, no rules firing yet) * - { ok: false, error }: network failure, 5xx, parse error → show error banner * + * `forbidden` is set when the server replied 403 (caller is not a moderator). + * The dashboard renders a dedicated "moderators only" notice for this case + * instead of the "Telemetry API unreachable" retry banner — a non-mod viewing + * the Observatory custom post is not an outage (App Review fix 2026-06-07). + * * Replaces the prior "return [] on any failure" pattern (Codex review HIGH F5) * where a backend outage was indistinguishable from "no events yet." */ export type ApiResult = | { ok: true; empty: false; data: T } | { ok: true; empty: true } - | { ok: false; error: string }; + | { ok: false; error: string; forbidden?: boolean }; export type ConfigRaw = { content: string; revisionId: string | null; isDefaultTemplate: boolean }; export type SaveResult = { rev: number; ruleCount: number }; diff --git a/src/lib/authFailToast.ts b/src/lib/authFailToast.ts new file mode 100644 index 0000000..18d913e --- /dev/null +++ b/src/lib/authFailToast.ts @@ -0,0 +1,32 @@ +/** + * Map a requireModerator() failure to user-facing toast text for Devvit + * menu + form handlers. + * + * Extracted from forms.ts (2026-06-07) so the mod-menu handlers in menu.ts + * can reuse the EXACT same wording when their defense-in-depth + * requireModerator() gate fails. Single source of truth — two copies of this + * string WOULD diverge on the next reword (same anti-drift rationale as the + * openaiErrors.ts + requireModerator.ts extractions). + * + * Reddit App Review (2026-06-07) flagged that every mod-only action must + * verify moderator status server-side, not lean on `forUserType: moderator` + * alone. The menu handlers now gate with requireModerator() and surface this + * toast on failure. + * + * Why 503/500 get distinct copy (AE Polish #10): a transient mod-check RPC + * blip (retryable) is split from a genuine "you're not a mod" (terminal) so + * an actual mod whose auth check 5xx'd is told to retry, not told they lack + * permission. + */ +export function authFailToast( + status: 401 | 403 | 500 | 503, + actionLabel: string, +): string { + if (status === 503) { + return `Mod check temporarily unavailable. Retry in ~30s, then ${actionLabel}.`; + } + if (status === 500) { + return `Mod check failed. See logs, then ${actionLabel}.`; + } + return `Mod-only action. Only this sub's moderators can ${actionLabel}.`; +} diff --git a/src/routes/forms.ts b/src/routes/forms.ts index e9753c4..48bc94b 100644 --- a/src/routes/forms.ts +++ b/src/routes/forms.ts @@ -30,6 +30,7 @@ import { getRecentSample } from '../core/recentSample'; import { explainRule, formatExplainToast } from '../core/explainRule'; import { setOpenaiKey } from '../state/apiKeyStore'; import { requireModerator } from '../lib/requireModerator'; +import { authFailToast } from '../lib/authFailToast'; import { checkRateLimit } from '../lib/ratelimit'; import { checkCircuit, recordFailure, recordSuccess } from '../lib/circuitBreaker'; import { log } from '../lib/log'; @@ -39,27 +40,6 @@ import type { AppConfig } from '../shared/types'; export const forms = new Hono(); -/** - * Map requireModerator() failure to user-facing toast text. - * - * Why: AE Polish #10 added a 503 (transient mod-check failure) status to - * distinguish "Reddit RPC blip — retry" from "you're not a mod." Forms - * previously surfaced the same "Mod-only action" toast for every failure, - * which lies to actual mods when their auth check fails to a 5xx. - */ -function authFailToast( - status: 401 | 403 | 500 | 503, - actionLabel: string, -): string { - if (status === 503) { - return `Mod check temporarily unavailable. Retry in ~30s, then ${actionLabel}.`; - } - if (status === 500) { - return `Mod check failed. See logs, then ${actionLabel}.`; - } - return `Mod-only action. Only this sub's moderators can ${actionLabel}.`; -} - interface FetchedPost { id?: string; title?: string; diff --git a/src/routes/menu.ts b/src/routes/menu.ts index 10ef3a1..fd5026d 100644 --- a/src/routes/menu.ts +++ b/src/routes/menu.ts @@ -16,6 +16,8 @@ import { loadFromWiki, WIKI_PAGE } from '../core/configSource'; import { K } from '../state/keys'; import { logModActivity, type ModActivityKind } from '../state/modActivity'; import { log } from '../lib/log'; +import { requireModerator } from '../lib/requireModerator'; +import { authFailToast } from '../lib/authFailToast'; async function logMenuAction(kind: ModActivityKind, detail?: string): Promise { try { @@ -37,15 +39,14 @@ export const menu = new Hono(); menu.post('/reload-config', async (c) => { await c.req.json(); - let subName: string; - try { - subName = (await reddit.getCurrentSubreddit()).name; - } catch (err) { - log.error('cm/menu/reload-config', 'could not resolve current sub', { err }); - return c.json({ - showToast: 'Could not resolve current subreddit — try again.', - }); - } + // App Review (2026-06-07): mod-only ACTIONS must verify mod status + // server-side, not rely on the menu's forUserType alone — this endpoint is + // HTTP-reachable by any custom-post viewer. requireModerator also resolves + // the sub, so it replaces the separate getCurrentSubreddit call (a 5xx/blip + // there now surfaces as the same transient-retry toast via authFailToast). + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'reload the config') }); + const subName = auth.sub; const loaded = await loadFromWiki(subName); if (!loaded.ok) { @@ -83,10 +84,13 @@ menu.post('/reload-config', async (c) => { menu.post('/recent-actions', async (c) => { try { await c.req.json(); + // App Review (2026-06-07): gate post-creation behind a server-side mod + // check (defense-in-depth beyond the menu's forUserType). + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'open the Observatory dashboard') }); log.info('cm/menu/recent-actions', 'creating Observatory post'); - const subreddit = await reddit.getCurrentSubreddit(); const post = await reddit.submitCustomPost({ - subredditName: subreddit.name, + subredditName: auth.sub, title: 'ContextMod Observatory', entry: 'default', textFallback: { @@ -119,6 +123,8 @@ menu.post('/recent-actions', async (c) => { menu.post('/set-openai-key', async (c) => { await c.req.json(); + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'set the OpenAI key') }); return c.json({ showForm: { name: 'setOpenaiKey', @@ -143,6 +149,8 @@ menu.post('/set-openai-key', async (c) => { menu.post('/explain-rule', async (c) => { await c.req.json(); + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'use AI rule explanations') }); return c.json({ showForm: { name: 'explainRule', @@ -168,6 +176,8 @@ menu.post('/explain-rule', async (c) => { menu.post('/simulate-rule', async (c) => { await c.req.json(); + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'simulate rules') }); return c.json({ showForm: { name: 'simulateRule', @@ -193,6 +203,8 @@ menu.post('/simulate-rule', async (c) => { menu.post('/test-rules', async (c) => { const evt = await c.req.json(); + const auth = await requireModerator(); + if (!auth.ok) return c.json({ showToast: authFailToast(auth.status, 'dry-run rules') }); log.info('cm/menu/test-rules', 'opened', { targetId: evt.targetId }); if (!evt.targetId) { return c.json({ diff --git a/tests/client/api.test.ts b/tests/client/api.test.ts index c989c27..d2274af 100644 --- a/tests/client/api.test.ts +++ b/tests/client/api.test.ts @@ -91,6 +91,27 @@ describe('fetchRecentSafe', () => { if (!result.ok) expect(result.error).toContain('404'); }); + it('App Review fix: 403 → ok:false forbidden:true (caller is not a moderator)', async () => { + mockFetch({ + ok: false, + status: 403, + json: () => Promise.resolve({ error: 'not a moderator of this sub', events: [] }), + }); + const result = await fetchRecentSafe(); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.forbidden).toBe(true); + expect(result.error).toContain('not a moderator'); + } + }); + + it('App Review fix: non-403 error → forbidden falsy (only 403 gates the dashboard)', async () => { + mockFetch({ ok: false, status: 503 }); + const result = await fetchRecentSafe(); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.forbidden).toBeFalsy(); + }); + it('Polish #21: 503 + server error body → surfaces server message + code', async () => { // /api/recent returns this exact shape when reddit.getCurrentSubreddit throws. mockFetch({ @@ -226,6 +247,17 @@ describe('fetchStatsSafe', () => { if (!result.ok) expect(result.error).toContain('500'); }); + it('App Review fix: 403 → ok:false forbidden:true', async () => { + mockFetch({ + ok: false, + status: 403, + json: () => Promise.resolve({ error: 'not a moderator of this sub', counters: {} }), + }); + const result = await fetchStatsSafe(); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.forbidden).toBe(true); + }); + it('network failure → ok:false', async () => { mockFetchReject(new Error('NetworkError')); const result = await fetchStatsSafe(); diff --git a/tests/client/app.test.tsx b/tests/client/app.test.tsx index 56ca90f..312f162 100644 --- a/tests/client/app.test.tsx +++ b/tests/client/app.test.tsx @@ -20,7 +20,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { render, screen, waitFor, act } from '@testing-library/react'; +import { render, screen, waitFor, act, within } from '@testing-library/react'; const fetchRecentSafe = vi.fn(); const fetchStatsSafe = vi.fn(); @@ -175,6 +175,42 @@ describe('App.tsx demo branch (Polish #34)', () => { }); }); +describe('App.tsx moderators-only gate (App Review fix 2026-06-07)', () => { + // Queries are scoped to each test's own `container` via within() — this + // suite has no testing-library auto-cleanup, so a global `screen` query + // would pick up a prior test's un-unmounted DOM. + it('renders "Moderators only" heading + NO error banner when the API 403s', async () => { + fetchRecentSafe.mockResolvedValue({ + ok: false, + error: 'HTTP 403: not a moderator of this sub', + forbidden: true, + }); + fetchStatsSafe.mockResolvedValue({ + ok: false, + error: 'HTTP 403: not a moderator of this sub', + forbidden: true, + }); + const { container } = render(); + await waitFor(() => { + expect(within(container).getByRole('heading', { name: /moderators only/i })).toBeTruthy(); + }); + // The misleading "Telemetry API unreachable" retry banner must NOT show + // for a 403 — a non-mod viewing the post is an intentional, gated state. + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(within(container).queryByText(/telemetry api unreachable/i)).toBeNull(); + }); + + it('a non-forbidden error (503) does NOT trigger the moderators-only gate', async () => { + fetchRecentSafe.mockResolvedValue({ ok: false, error: 'HTTP 503' }); + fetchStatsSafe.mockResolvedValue({ ok: false, error: 'HTTP 503' }); + const { container } = render(); + await waitFor(() => { + expect(within(container).getAllByText(/contextmod/i).length).toBeGreaterThan(0); + }); + expect(within(container).queryByRole('heading', { name: /moderators only/i })).toBeNull(); + }); +}); + // AE Polish #98: structural CLS smoke for Polish #62. Polish #62 // closed CLS=0.328 (POOR) -> 0.04 (GOOD) by rendering StatsRow with // `stats ?? ZERO_STATS` so the 4-card grid mounts at first paint diff --git a/tests/lib/authFailToast.test.ts b/tests/lib/authFailToast.test.ts new file mode 100644 index 0000000..d28eb5d --- /dev/null +++ b/tests/lib/authFailToast.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from 'vitest'; +import { authFailToast } from '../../src/lib/authFailToast'; + +/** + * Pins the toast wording shared by forms.ts + menu.ts (extracted 2026-06-07 + * for the App Review mod-permission fix). A reword in one call site can no + * longer silently diverge from the other. + */ +describe('authFailToast', () => { + it('403 (not a mod) → terminal "mod-only" message naming the action', () => { + const msg = authFailToast(403, 'reload the config'); + expect(msg).toMatch(/mod-only/i); + expect(msg).toContain('reload the config'); + }); + + it('401 (unauthenticated) → same terminal "mod-only" message', () => { + expect(authFailToast(401, 'reload the config')).toMatch(/mod-only/i); + }); + + it('503 (transient mod-check blip) → retry message, NOT a permission denial', () => { + const msg = authFailToast(503, 'simulate rules'); + expect(msg).toMatch(/temporarily unavailable|retry/i); + expect(msg).not.toMatch(/mod-only/i); + expect(msg).toContain('simulate rules'); + }); + + it('500 (mod-check failure) → server-error message, NOT a permission denial', () => { + const msg = authFailToast(500, 'set the OpenAI key'); + expect(msg).toMatch(/failed/i); + expect(msg).not.toMatch(/mod-only/i); + }); +}); diff --git a/tests/routes/menu-auth.test.ts b/tests/routes/menu-auth.test.ts new file mode 100644 index 0000000..6df5c3f --- /dev/null +++ b/tests/routes/menu-auth.test.ts @@ -0,0 +1,169 @@ +/** + * App Review mod-permission regression (2026-06-07). + * + * Reddit App Review rejected cm-devvit 0.3.0: "Any mod-only actions should + * check for proper mod perms before allowing the action to be submitted." + * The /internal/menu/* handlers previously relied ONLY on the menu item's + * `forUserType: "moderator"` — but the handler endpoints are HTTP-reachable + * by any viewer of the Observatory custom post (same reachability the + * /api/* + form-submit handlers already defend against). These tests pin a + * server-side requireModerator() gate on every menu handler so a future + * refactor that drops one breaks CI. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const requireModeratorMock = vi.fn(); +const loadFromWiki = vi.fn(); +const publish = vi.fn(); +const submitCustomPost = vi.fn(); +const getCurrentSubreddit = vi.fn(async () => ({ name: 'r_test' })); +const getCurrentUser = vi.fn(async () => ({ username: 'mod_alice' })); +const logModActivity = vi.fn(); +const redisSet = vi.fn(); + +vi.mock('@devvit/web/server', () => ({ + reddit: { + getCurrentSubreddit: () => getCurrentSubreddit(), + getCurrentUser: () => getCurrentUser(), + submitCustomPost: (...a: unknown[]) => submitCustomPost(...a), + }, + redis: { set: (...a: unknown[]) => redisSet(...a) }, +})); +vi.mock('../../src/lib/requireModerator', () => ({ + requireModerator: () => requireModeratorMock(), +})); +vi.mock('../../src/core/configSource', () => ({ + loadFromWiki: (...a: unknown[]) => loadFromWiki(...a), + WIKI_PAGE: 'botconfig/contextmod', +})); +vi.mock('../../src/state/configStore', () => ({ + publish: (...a: unknown[]) => publish(...a), +})); +vi.mock('../../src/state/modActivity', () => ({ + logModActivity: (...a: unknown[]) => logModActivity(...a), +})); +vi.mock('../../src/state/keys', () => ({ + K: { cfgLastWikiRev: (sub: string) => `cm:cfg:lastwiki:${sub}` }, +})); + +import { menu } from '../../src/routes/menu'; + +const NON_MOD = { ok: false as const, status: 403 as const, error: 'not a moderator of this sub' }; +const TRANSIENT = { + ok: false as const, + status: 503 as const, + error: 'mod check transient failure (retry in ~30s): ECONNRESET', +}; +const AS_MOD = { ok: true as const, sub: 'r_test', username: 'mod_alice' }; + +const ALL_ENDPOINTS = [ + '/reload-config', + '/recent-actions', + '/set-openai-key', + '/explain-rule', + '/simulate-rule', + '/test-rules', +]; + +async function postMenu(path: string, body: unknown = {}): Promise { + return menu.request( + new Request(`http://x${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + getCurrentSubreddit.mockResolvedValue({ name: 'r_test' }); + getCurrentUser.mockResolvedValue({ username: 'mod_alice' }); +}); + +describe('mod-menu handlers reject non-mods (App Review fix 2026-06-07)', () => { + for (const path of ALL_ENDPOINTS) { + it(`${path}: non-mod → 200 UiResponse with a mod-only toast, never a form/navigate`, async () => { + requireModeratorMock.mockResolvedValue(NON_MOD); + const res = await postMenu(path); + // Menu handlers return a 200 UiResponse; the rejection is surfaced as a + // toast (Devvit's contract), and crucially the action never executes. + expect(res.status).toBe(200); + const json = (await res.json()) as { + showToast?: string; + showForm?: unknown; + navigateTo?: string; + }; + expect(json.showToast).toMatch(/mod-only/i); + expect(json.showForm).toBeUndefined(); + expect(json.navigateTo).toBeUndefined(); + }); + + it(`${path}: transient mod-check (503) → retry toast, NOT a permission denial`, async () => { + requireModeratorMock.mockResolvedValue(TRANSIENT); + const res = await postMenu(path); + const json = (await res.json()) as { showToast?: string }; + expect(json.showToast).toMatch(/temporarily unavailable|retry/i); + expect(json.showToast).not.toMatch(/mod-only/i); + }); + } + + it('reload-config: a non-mod never reads the wiki or publishes config', async () => { + requireModeratorMock.mockResolvedValue(NON_MOD); + await postMenu('/reload-config'); + expect(loadFromWiki).not.toHaveBeenCalled(); + expect(publish).not.toHaveBeenCalled(); + expect(redisSet).not.toHaveBeenCalled(); + }); + + it('recent-actions: a non-mod never creates a custom post', async () => { + requireModeratorMock.mockResolvedValue(NON_MOD); + await postMenu('/recent-actions'); + expect(submitCustomPost).not.toHaveBeenCalled(); + }); +}); + +describe('mod-menu handlers allow mods (happy path stays intact)', () => { + it('reload-config: a mod triggers loadFromWiki + publish, gets a rule-count toast', async () => { + requireModeratorMock.mockResolvedValue(AS_MOD); + loadFromWiki.mockResolvedValue({ + ok: true, + config: { runs: [{ checks: [{ rules: [{}, {}] }] }] }, + revisionId: 'rev9', + }); + publish.mockResolvedValue(7); + const res = await postMenu('/reload-config'); + expect(res.status).toBe(200); + const json = (await res.json()) as { showToast: string }; + expect(loadFromWiki).toHaveBeenCalledWith('r_test'); + expect(publish).toHaveBeenCalledWith({ runs: [{ checks: [{ rules: [{}, {}] }] }] }, 'r_test'); + expect(json.showToast).toMatch(/Loaded 2 rules \(rev 7\)/); + }); + + it('recent-actions: a mod creates the Observatory post + gets a navigateTo', async () => { + requireModeratorMock.mockResolvedValue(AS_MOD); + submitCustomPost.mockResolvedValue({ id: 't3_post', permalink: '/r/r_test/comments/x/' }); + const res = await postMenu('/recent-actions'); + expect(res.status).toBe(200); + const json = (await res.json()) as { navigateTo?: string }; + expect(submitCustomPost).toHaveBeenCalledWith( + expect.objectContaining({ subredditName: 'r_test', entry: 'default' }) + ); + expect(json.navigateTo).toContain('/r/r_test/comments/x/'); + }); + + it('set-openai-key: a mod sees the form', async () => { + requireModeratorMock.mockResolvedValue(AS_MOD); + const res = await postMenu('/set-openai-key'); + const json = (await res.json()) as { showForm?: { name: string } }; + expect(json.showForm?.name).toBe('setOpenaiKey'); + }); + + it('test-rules: a mod with a targetId sees the dry-run form', async () => { + requireModeratorMock.mockResolvedValue(AS_MOD); + const res = await postMenu('/test-rules', { targetId: 't3_abc' }); + const json = (await res.json()) as { showForm?: { name: string } }; + expect(json.showForm?.name).toBe('testRules'); + }); +}); diff --git a/tests/routes/menu-test-rules.test.ts b/tests/routes/menu-test-rules.test.ts index 5f6b5b7..9f7b325 100644 --- a/tests/routes/menu-test-rules.test.ts +++ b/tests/routes/menu-test-rules.test.ts @@ -13,6 +13,11 @@ vi.mock('../../src/core/configSource', () => ({ WIKI_PAGE: 'botconfig/contextmod', })); vi.mock('../../src/state/configStore'); +// menu handlers now gate on requireModerator (App Review fix 2026-06-07); +// mock it as a mod so these handler-shape tests still exercise the form path. +vi.mock('../../src/lib/requireModerator', () => ({ + requireModerator: vi.fn(async () => ({ ok: true, sub: 'r_test', username: 'mod_alice' })), +})); import { menu } from '../../src/routes/menu';