fix: Move NCMEC routing of reports to env config

[juanmrad]

Context & Requests for Reviewers

Fix bug coming from SaaS era. Move NCMEC routing of reports and display to an env variable to determine whether submissions go to NCMEC's sandbox (exttest.cybertip.org) or production (report.cybertip.org).

Previously, this was gated by a hardcoded allowlist of MRT queue UUIDs in server/services/ncmecService/ncmecService.ts (ncmecProdQueues). Adding a new tenant required a code change. That doesn't make sense for a self-hosted OSS project each deployment running Coop, not the maintainers, should decide whether their deployment talks to prod.

Summary by CodeRabbit

• Documentation

  • Clarified NCMEC integration setup, credential distinctions, manual review queue configuration, and expanded access/roles and settings tables. Improved examples and response-contract guidance: responses must include entries for every requested user/media; if all media are marked missing the job is treated as a permanent error and will not be retried.

• Configuration Enhancement

  • Added NCMEC_ENV to control submission endpoint: set to "production" for live reporting; otherwise uses the sandbox/test endpoint.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f80cb85-bde3-4546-ad10-1fb1c8745e1f

📥 Commits

Reviewing files that changed from the base of the PR and between c15d1be and 9048fee.

📒 Files selected for processing (1)

• docs/NCMEC.md

✅ Files skipped from review due to trivial changes (1)

• docs/NCMEC.md

---

📝 Walkthrough

Walkthrough

This PR centralizes NCMEC test/production endpoint selection under an environment variable, NCMEC_ENV; updates docs to clarify NCMEC APIs, webhook contracts, and error semantics; removes the exported ncmecProdQueues constant; and adapts manual-review wiring to use the new environment-based routing.

Changes

NCMEC environment-based routing

Layer / File(s)	Summary
Documentation: NCMEC setup, behavior, and contracts docs/NCMEC.md	Rewrites prerequisites to distinguish Hash Sharing vs CyberTipline credentials, reformats NCMEC Settings and Access/Roles tables, adjusts routing/rule setup bullets, changes Additional Info request example shape, adds a detailed Additional Info response fields table, and documents error behavior (Coop errors when responses omit requested entries; job permanently errors if all media are missing: true).
Environment config and typing server/.env.example, server/decs.d.ts	Adds NCMEC_ENV to .env.example with sandbox-by-default guidance and documents that NCMEC_ENV=production enables live submissions; declares NCMEC_ENV?: string on NodeJS.ProcessEnv.
Service exports cleanup server/services/ncmecService/index.ts, server/services/ncmecService/ncmecService.ts	Removes the exported ncmecProdQueues constant from the ncmec service surface and adjusts a minor formatting change in updateNcmecOrgSettings assignment.
Manual-review wiring and routing logic server/iocContainer/index.ts	Removes queueId destructuring from the manual review decision handler, drops importing ncmecProdQueues, and replaces queue-list-based test detection with process.env.NCMEC_ENV !== 'production' when deciding test vs production endpoint for submitReport.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through docs and env today,
Swapped queues for a name that lights the way.
Sandbox sleeps until production sings,
Tables, responses, and clearer strings.
A tiny hop, a safer spring!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description provides context explaining the bug fix and rationale, but lacks required sections for Tests and Rollout Plan as specified in the template.	Add a Tests section describing how changes were validated, and optionally a Rollout Plan section noting any deployment considerations.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: moving NCMEC routing from a hardcoded allowlist to environment variable configuration.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch move-NCMEC-routing-controls-to-env-variable

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

server/iocContainer/index.ts (1)

1127-1256: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wrap this switch clause in braces to avoid lexical declaration hazards.

Line 1205 declares const isTest directly under the case 'SUBMIT_NCMEC_REPORT': label. This violates noSwitchDeclarations and creates scope hazards. The entire case body contains multiple lexical declarations (lines 1133, 1145, 1152, 1157, 1158, 1205, 1226, 1232) that all require block scope. Wrap the case body in braces.

Suggested fix

-              case 'SUBMIT_NCMEC_REPORT':
+              case 'SUBMIT_NCMEC_REPORT': {
                 if (job.payload.kind !== 'NCMEC') {
                   throw new Error(
                     'Attempting to submit a NCMEC report for a non-NCMEC job',
                   );
                 }
@@
                 if (
                   actionAndPolicy != null &&
                   actionAndPolicy.actionsToRunIds != null &&
                   isNonEmptyArray(decisionActions) &&
                   !isTest
                 ) {
                   await publishActions({
                     decisionActions,
                     policyIds: actionAndPolicy.policyIds,
                     orgId,
                     item: job.payload.item,
                     actorId: reviewerId,
                     actorEmail: reviewerEmail,
                   });
                 }
                 break;
+              }

🤖 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 `@server/iocContainer/index.ts` around lines 1127 - 1256, The switch case for
'SUBMIT_NCMEC_REPORT' contains multiple lexical declarations (e.g., const
itemType, displayName, profilePicUrl, media, createdAt, isTest) and should be
wrapped in a block to avoid lexical-declaration hazards; fix by adding braces
after the case label and before the corresponding break so the entire case body
(the code that builds media via decision.reportedMedia, calls
container.getItemTypeEventuallyConsistent, container.NcmecService.submitReport,
container.NcmecService.getNCMECActionsToRunAndPolicies, and publishActions) is
scoped within { ... }.

🤖 Prompt for all review comments with 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.

Inline comments:
In `@docs/NCMEC.md`:
- Line 32: Fix the broken Markdown in the bullet that starts with **A dedicated
NCMEC manual review queue named "NCMEC Review** by closing the bold and the
quote; update the text so the quoted queue name is properly closed (e.g., change
to **A dedicated NCMEC manual review queue named "NCMEC Review"**:) so the
bullet renders correctly alongside references like NCMEC_ENV.

---

Outside diff comments:
In `@server/iocContainer/index.ts`:
- Around line 1127-1256: The switch case for 'SUBMIT_NCMEC_REPORT' contains
multiple lexical declarations (e.g., const itemType, displayName, profilePicUrl,
media, createdAt, isTest) and should be wrapped in a block to avoid
lexical-declaration hazards; fix by adding braces after the case label and
before the corresponding break so the entire case body (the code that builds
media via decision.reportedMedia, calls
container.getItemTypeEventuallyConsistent, container.NcmecService.submitReport,
container.NcmecService.getNCMECActionsToRunAndPolicies, and publishActions) is
scoped within { ... }.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 01761243-edc0-47d5-9d1a-1c979a4fa95e

📥 Commits

Reviewing files that changed from the base of the PR and between 683d0fb and c15d1be.

📒 Files selected for processing (6)

• docs/NCMEC.md
• server/.env.example
• server/decs.d.ts
• server/iocContainer/index.ts
• server/services/ncmecService/index.ts
• server/services/ncmecService/ncmecService.ts

💤 Files with no reviewable changes (1)

• server/services/ncmecService/index.ts

[julietshen]

good catch 😭