[i,l,-r] Phase 2b/3 + security hardening: reviewer picker UX + opt-out copy-link

[phosphore]

End-to-end change covering both halves of the picker UX in one PR. Default behaviour stays identical to Phase 2a (no picker selection → notify all qualified peers), so this is safe to deploy even before the frontend opts users into the picker.

The PR now also carries a security-hardening pass that addresses every P0/P1/P2 finding from the senior-security-engineer review (modulo three explicitly accepted: P1-5, P2-12, P2-14). See the Security hardening section below.

Backend

ProposalHandler interface

Now takes a ProposeOptions record carrying two per-request flags:

• reviewerFilter — narrow recipients to a requester-selected subset. JoinOperation.propose intersects with the policy ACL so a stale picker selection naming a non-approver is silently dropped, never widened.
• notifyReviewers — when false, the JWT is generated and signed, but onOperationProposed is skipped. Requester gets the approval URL to share manually.

Upstream 2-arg propose is now a default method delegating to the 3-arg form, so existing subclasses (Debug/MailProposalHandler) and tests don't break.

JoinOperation.propose

Gains a Set<EndUserId> filter overload. The 1-arg upstream form delegates with null. Filter is applied to the APPROVE_OTHERS-derived set; group principals pass through (expanded later by the proposal handler).

GroupsResource

• New GET /reviewers returns candidate peer reviewers with a suggested flag for those sharing an approver group with the requester. Backed by ReviewerCandidates helper that uses 2+M API calls (listMembershipsByUser on requester + group expansion on shared approver groups) instead of O(N) per-candidate lookups — important when ACLs name broad groups like engineering@.
• POST now extracts selectedReviewers[] and notifyReviewers form fields and threads them through ProposeOptions.
• JoinInfo gains approvalUrl + copyLinkEnabled, both gated by new SLACK_COPY_LINK_ENABLED env var.

Frontend (vanilla JS + jQuery + MDC, upstream style)

GroupInfoView._bind fetches /reviewers and renders a checkbox list under the join form when status is JOIN_ALLOWED_WITH_APPROVAL:

• Suggested teammates are pre-checked + highlighted with a "team" badge
• Search input filters list client-side
• When copyLinkEnabled=true: "Notify selected reviewers in Slack" toggle (hidden+checkbox pair) + copy-able approvalUrl box on submit response with Clipboard API + text-select fallback

If /reviewers fails the picker silently degrades — form still submits, backend defaults to notifying every qualified peer (= Phase 2a behaviour).

Security hardening (P0/P1/P2 findings)

ID	Fix
P0-1	SlackMessages: justification rendered as PlainTextObject (mrkdwn-inert), capped at 2.5 KB with visible [truncated] suffix. Inputs.copyValues: defense-in-depth absolute per-input cap (INPUT_MAX_LENGTH=16384) at the REST boundary so a misconfigured/unbounded policy can't ship multi-MB justifications.
P1-4	SlackProposalHandler: Semaphore-gated per-reviewer fan-out (Options.maxConcurrentFanOut, default 8) bounds chat.postMessage pressure on Slack Tier 4. GroupsResource.getReviewers: per-user Guava RateLimiter (1 req/s, ~1s burst) backing GET /reviewers so a polling client can't burn the SA's Cloud Identity quota.
P1-6	Catalog-side defense-in-depth in JitGroupContext.JoinOperation.propose: when the policy ACL has no GroupId approver, reject reviewerFilter entries that aren't in the direct ACL. ACL-with-group still trusts the REST layer's expansion check.
P1-7	GroupResolver.expand Javadoc: visibility comment + caller-audit list, emphasising it is not itself an authorisation decision.
P1-8 / P2-10	OperationAuditTrail.joinProposed: 3-arg overload taking notifyReviewers. Emits a new proposal/notify_reviewers label and "via copy-link" message body when false, so the BigQuery sink can slice approval requests not visible to the normal reviewer set.
P2-9	parseSelectedReviewers regex ^[^@\s]+@[^@\s]+\.[^@\s]+$ rejects @@@@, multi-@ strings, embedded whitespace, missing TLDs that the previous contains("@") check accepted.
P2-11	SlackMessageRegistry.requestKey: HMAC-SHA-256 (was bare SHA-256) under a Secret Manager-backed salt (SLACK_REGISTRY_KEY_SALT_SECRET). requestKey is now an instance method; constructor rejects a blank salt. The wavemm-iam parent PR provisions the secret + IAM grant.
P2-13	application.properties: pin com.slack.api log level to INFO so a future DEBUG-everything operator can't leak the Bearer xoxb-… token into Cloud Logging. pom.xml: OWASP dependency-check 11.1.1 plugin (failBuildOnCVSS=8, unbound to default phases).

P1-3 (bot-token rotation runbook) and P0-2 (Firestore IAM scope honest-comment) live in the wavemm-iam parent PR alongside the matching Terraform.

Explicitly skipped per maintainer call: P1-5 (PII redaction in logs), P2-12 (dual-approver race test), P2-14 (0-test-mpa-flow cleanup).

Build

pom.xml → 2.3.0-wavemm.6. Forces new App Engine version_id so env-var-only changes (e.g. flipping SLACK_COPY_LINK_ENABLED) propagate.

Tests updated

• TestGroupsResource: new injected options field, 3-arg propose Mockito stubs, rate-limit + email-validation regression tests.
• TestAbstractProposalHandler: 2-arg JoinOperation.propose mock signature.
• TestSlackMessages: PlainTextObject + truncation regression tests.
• TestSlackMessageRegistry: HMAC salt-distinguishes-keys + blank-salt-rejection tests.
• TestSlackProposalHandler: concurrency-cap + Options-validation tests.
• TestJitGroupContext: catalog-side filter validation tests (direct-ACL reject + group-ACL trust).
• TestOperationAuditTrail: copy-link branch label + message body tests.
• TestInputs: per-input length cap test.

1008/1008 unit tests green locally (mvn -DskipITs test).

Existing test assertions about JoinInfo keep passing — new fields default to null/false on legacy paths.

Companion PR

Parent-repo bump + Terraform changes: wavemm/wavemm-iam#290

🤖 Generated with Claude Code