fix: harden agent report-access program (post-merge review of #258)#262
Merged
important-new merged 2 commits intoJul 22, 2026
Merged
Conversation
…orHub#258) Follow-up fixes from a review of the merged people/role-profiles + role-aware sending + agent unified-link program (InspectorHub#258): - SMS consent gate now keys on the per-recipient role, so a recipientKind=all rule can no longer text the client without recorded consent (TCPA) - primary-client add is an atomic insert-if-no-client (closes a TOCTOU race) - tenant /login excludes global-agent rows (no member lockout on a shared email) - resolveRecipients guards listPeople so a transient DB error can't silently drop every report delivery - report-token sign-in is emailed to the agent's own inbox instead of returned to the caller (closes a report-link -> full agent-session takeover) - automation editor warns when recipients = "everyone on the inspection" - analytics agent-name lookup is scoped to rendered rows and chunked under the D1 bind-parameter cap - removePerson is scoped to the URL inspection id - find-my-report redeem prefers a client session when the email holds a client-kind grant (agents still route to the dashboard) - misc: reactivate 409 mapping, cross-tenant template-id rejection, idempotent role-profile seed, fail-closed capability default Adds regression tests for the SMS-all, login-exclusion, and dual-identity paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017YiCgWAmqyzpfs3pp1kjg3
…mailed link
The magic-login/request response changed from { loginUrl } to { sent: true }
(the single-use link is emailed to the agent now, not returned), which drifted
the committed OpenAPI snapshot and broke the e2e that asserted the old
return-and-navigate flow.
- regenerate server/lib/mcp/openapi-snapshot.json
- rewrite Scenario 1 + Scenario 4 to the emailed-link flow: request -> { sent }
-> read the single-use link from the E2E email sink -> redeem -> /agent-dashboard
(Scenario 4 waits for a code that differs from an earlier scenario's email)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YiCgWAmqyzpfs3pp1kjg3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up hardening from a systematic post-merge review of the agent report-access program (#258 — people/role profiles + role-aware sending + agent unified link). Four independent reviewers swept the merged diff (data foundation, role-aware sending, agent auth, downstream refactor); this PR fixes the verified findings.
Security / correctness
recipientKind, so arecipientKind='all'rule texted the client with no recorded consent. Now keyed on the per-recipientlog.recipientRoleKey, covering bothroleandall.addPersondid SELECT-then-INSERT with no constraint, so two concurrent adds of different client contacts both landed. Now an atomicINSERT … WHERE NOT EXISTS(D1 serializes writes); the friendly 409 is preserved./login:validateCredentialsselected by email only, so a global agent (role='agent',tenant_id IS NULL) could authenticate through the tenant front door and shadow a member sharing the email. Now excludes global-agent rows.POST /api/agent/magic-login/requestreturned the single-use login URL to the caller, so a forwarded/leaked durable report link could be replayed into a full agent session. The link is now emailed to the agent's own inbox; the endpoint returns{ sent: true }(anti-oracle). UI shows a "check your email" confirmation.resolveRecipients: an unguardedlistPeoplebroke the documented never-throws contract — a transient DB error aborted the whole fan-out with no retry. Now guarded.Robustness / data
removePersonis scoped to the URL inspection id (was tenant+id only).recipientKind='all'shows an editor warning (it includes any listing agent on the inspection).capabilitiesForKinddefault.Tests
Regression tests for the SMS-
allbypass, the login exclusion, and the dual-identity redeem (both directions); existing SMS/magic-login/component tests updated to the new contracts. Full gate green: type-check,test:unit,test:web,test:workers,db:check,lint.🤖 Generated with Claude Code