{
"command": "pwd"
}/Users/warrenthompson/Source/LoginID/02-ai-security-architecture
{
"file_path": "/Users/warrenthompson/Source/LoginID/CLAUDE.md"
}1 # LoginID Take-Home POC — Project Root
2
3 ## Role
4
5 You are the project manager agent for this repository. This file is the shared context every track refers back to. Each of the five track directories below has its own CLAUDE.md that can be worked independently by a scoped subagent, but that subagent should read this root file first for the assignment, the overall narrative, and how its track fits with the other four.
6
7 When you spin up agents or subagents to do work in a track, point them at that track's CLAUDE.md as their brief, and tell them this root file exists for cross-track context. Don't let a track's agent silently duplicate scope another track owns — check the "Owns / does not own" boundaries below first.
8
9 ## What this repository is
10
11 This is a take-home assignment for a LoginID interview, submitted as a full directory. The goal is not just to answer three coding questions — it's to demonstrate capability as an AI architect: how you scope a problem, use AI tooling deliberately, reason about security and data, and produce something that reads as an intentional, well-run engineering effort rather than a quick script.
12
13 Code does not need to work or be fully complete. LoginID has said explicitly they care more about the design and the reasoning behind it, and about which AI tools/frameworks were used and how. Every track should document its reasoning, not just its output.
14
15 ## The assignment (verbatim from LoginID)
16
17 Provide code solutions for the following questions in Go (preferably) or other languages — describe any tool, framework, or AI used.
18
19 1. Code of a database DAO that can store and retrieve `user_profile` (name, address, phone) and `user_credential` (username, method, password), with support for multiple databases (PostgreSQL or CockroachDB, SQLite, etc).
20 2. Design a RESTful API web service to search and retrieve the above user profile data, plus security API authentication.
21 3. Design a service connector to third-party identity providers (referred to generically as ABC or XYZ) to retrieve personal data from vendors, with the following endpoints:
22 - `/auth` — retrieves an `access_token` based on a POST JSON body `{"username": "<string>", "password": "<string>"}`
23 - `/identity` — retrieves user PII (name, phone, address: `street_address`, `locality`, `region`, `postal_code`, `country`) based on a POST JSON body `{"phone": "<string>", "name": "<string>"}`
24
25 Go is the preferred language. Other languages are acceptable if justified.
26
27 ## Track structure
28
29 Five directories, each independently workable, each owning a distinct slice of the problem:
30
31 | Directory | Track | Owns |
32 |---|---|---|
33 | `01-product-industry-research-design/` | Product & Industry Research (incl. Design) | Market/industry framing (IAM, passwordless auth, IDP aggregation space), personas and use cases, API and data contract design rationale, UX of the surfaces that would sit on top of this |
34 | `02-ai-security-architecture/` | AI Architecture & Security Architecture | The AI tooling/workflow used to produce this submission, threat modeling, auth/authz design for the API, credential and PII handling, secrets management, third-party connector security |
35 | `03-engineering-delivery/` | Engineering & Delivery | The actual Go code for all three questions, project structure, test strategy, delivery documentation |
36 | `04-infra-devops/` | Infra & DevOps | How this would be built, containerized, deployed, and operated — CI/CD, environments, multi-database configuration, observability |
37 | `05-data-ops/` | Data Ops | Data modeling for `user_profile` / `user_credential`, multi-database abstraction strategy, migrations, PII data governance and retention |
38
39 Directories are numbered for reading order in the final submission, not for sequencing of work — tracks can be worked in any order or in parallel.
40
41 ## Working principles for every track
42
43 - State your reasoning, not just your conclusion. LoginID is grading the "why" as much as the "what."
44 - Name the AI tool/workflow you used for that track's output (this project is being built with Claude Code; say so, and say how — e.g., subagents scoped per track, a given skill, a given model).
45 - Keep each track's CLAUDE.md self-contained enough that a fresh agent with no other context can pick it up and do correct work from it alone, plus this root file.
46 - Cross-reference other tracks by relative path (e.g., `../05-data-ops/CLAUDE.md`) rather than restating their content.
47 - Do not write implementation code until Warren gives an explicit, project-wide go-ahead. Design docs, schemas described in prose, plans, and decision records are fine; `.go`, `.sql`, `go.mod`, Dockerfiles, and CI config are not.
48
49 ## Org, hiring, and orchestration
50
51 The project is run as an org: this PM agent (Dana Whitfield) orchestrates five standing track-lead agents, and each lead hires a small sub-team of specialist agents with deliberately varied personalities. **Each track lead runs as its own top-level Claude Code session**, launched from its track directory in its own tmux window — so it loads its own track `CLAUDE.md` and persona natively, owns and spawns its hires as named teammates in that window, and talks to the PM session through cross-session messaging. Hires belong to their lead's session, not the PM's. The governing files, all at the root:
52
53 - `PLAN.md` — the approved org plan for the planning phase (phases, gates, verification). `~/.claude/plans/` is scratch; this is the record.
54 - `PLANNING.md` — the thin cross-track board: one row per track, one writer per row, plus the service-boundaries section 03 owns and 04 reads.
55 - `CASTING.md` — archetype palette, org-wide variance rules, hire sizing, the persona-writing standard, the orchestration-pattern catalog (adversarial pair, red/blue, three hats, rotating devil's advocate, structured written debate, newcomer's question), the deep-research rule, the hire-creation procedure, and the registry of every hire.
56 - `LOG.md` — dated, append-only journal of what happened and why (team-lead writes it; it is part of the submission).
57 - `.claude/agents/` — one agent definition per hire (model pinned; a once-only per-agent `PreToolUse` hook displays its own persona on its first tool call). `.claude/AGENT_TEMPLATE.md` is the starting point (kept outside the agents directory so it is not registered as an agent type itself).
58 - `02-ai-security-architecture/planning-approach.md` — the agreed Claude-architecture approach: model/effort by role, coordination rules, hand-off standard, dependency order.
59
60 Rules that hold across every track:
61 - Every agent — lead or hire — has a profile-gen persona (name, temperament, professional history, technical background, stated blind spot). Hires are created text-first with `--output file`; portraits arrive later from `scripts/portrait-queue.sh`. Hires are **never** added as `profile-gen` markers in any `CLAUDE.md` — only the six markers that exist today.
62 - Hires do not spawn subagents. Only track leads (and team-lead) authorize or run deep research (patents, whitepapers, standards, primary sources); hires request it from their lead.
63 - Tracks never modify root-level state (this file, the root persona, root config); flag it to team-lead instead.
64 - Hand-offs are files by relative path, written so the receiver can act without re-deriving the sender's reasoning; chat messages signal readiness, they do not carry content.
65 - Edit `CLAUDE.md` files at turn boundaries only (prompt-cache stability). Milestone `git` commits at the end of each phase; no mid-task commits.
66
67 ## Status
68
69 Design phase complete for all five tracks (see each track's `CLAUDE.md` Status). All implementation code deliberately wiped 2026-09-12; the project is in the **planning phase** described in `PLAN.md`: Phase 0 scaffolding done, Phase 1 (leads plan in plan mode and propose hires) next, then hiring, in-team debates, a cross-track consistency pass, and Jira stories in project `LT`. No code until Warren's explicit go-ahead.
70
71 <!-- profile-gen:start slug=dana-whitfield -->
72 @profiles/dana-whitfield/dana-whitfield.md
73 <!-- profile-gen:end slug=dana-whitfield -->
74 {
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/CLAUDE.md"
}1 # Track: AI Architecture & Security Architecture 2 3 Refer back to `../CLAUDE.md` for the full assignment text and how this track fits with the other four. 4 5 ## Mission 6 7 Own two things that are related but distinct: (1) the story of how AI was used as an architecture and delivery tool across this whole submission, and (2) the security architecture for everything the engineering track builds — the DAO, the REST API, and the third-party IDP connector. 8 9 LoginID explicitly asked submitters to describe any tool, framework, or AI used — this track is where that answer lives in full, even though other tracks should each note their own AI usage briefly too. 10 11 ## Owns 12 13 - AI architecture narrative: what Claude Code was used for across the project, how the five-track structure itself was designed and orchestrated (root CLAUDE.md + per-track CLAUDE.md + scoped subagents), what was AI-generated vs. human-directed, and why that workflow was chosen. 14 - Threat modeling for the system: attacker-relevant risks around storing `user_credential` (usernames/passwords), storing and serving `user_profile` PII, and the third-party connector holding vendor `access_token`s. 15 - API authentication/authorization design for the REST service in question 2 — what "security API authentication" means here (e.g., API keys, OAuth2 client credentials, mTLS, or JWT bearer tokens), and why. 16 - Credential handling: password storage (hashing/salting strategy), token lifecycle for third-party `access_token`s from question 3, and secrets management for any credentials this service itself needs to call ABC/XYZ. 17 - Security review of the connector contracts in question 3: what happens to the third-party `access_token` once retrieved, how `/identity` calls are authenticated to the vendor, and what this service should and shouldn't log or persist from a PII/security standpoint. 18 19 ## Does not own 20 21 - The industry/product framing for why this system exists — that's `../01-product-industry-research-design/CLAUDE.md`. 22 - Writing the Go code itself — that's `../03-engineering-delivery/CLAUDE.md`, though this track's design should be specific enough to implement directly. 23 - Runtime secrets storage mechanics in a deployed environment (e.g., which secrets manager, which CI/CD injects what) — that's `../04-infra-devops/CLAUDE.md`; this track sets the requirement, infra/devops implements it. 24 - Data retention/governance policy for stored PII — that's `../05-data-ops/CLAUDE.md`; this track covers security of access and transit, not lifecycle/retention. 25 26 ## Deliverables 27 28 - A written AI-tooling/workflow summary for the whole project. 29 - A threat model covering credential storage, PII exposure, and third-party token handling. 30 - A concrete auth/authz design for the REST API in question 2. 31 - A security design for the IDP connector in question 3, including token handling and what gets logged. 32 33 ## AI tooling note 34 35 This track is itself where the project's AI-usage story is documented in depth — treat it as both a subject-matter track and the place other tracks' brief AI notes get rolled up if useful. See `ai-workflow-narrative.md` for the full write-up. 36 37 ## Status 38 39 Threat model, API auth/authz design, and connector security design are drafted: 40 41 - `threat-model.md` — STRIDE-style analysis of `user_credential`, `user_profile`, and third-party `access_token` exposure, per-asset mitigations. 42 - `api-auth-design.md` — OAuth2 client-credentials grant with short-lived JWT bearer tokens and scoped, object-level authorization for question 2's search/retrieve API. Reasoned against static API keys and mTLS-only alternatives. 43 - `connector-security.md` — token handling, vendor-call authentication, and logging/persistence rules for question 3's `/auth` and `/identity` connector. 44 - `ai-workflow-narrative.md` — the full AI-tooling/workflow story for the submission, building on `research-claude-architecture-best-practices.md`. 45 - `planning-approach.md` — team-wide recommendation (requested by team-lead) for model/effort pairings per role, planning-phase coordination pattern, cross-track dependency order, and the shape/location of the shared planning artifact. Synthesized from all four other tracks' input plus this track's own view. Two follow-ups for this track surfaced from it, deferred until the planning pause lifts: a receiver-shaped 02→03 hand-off (token scheme and validation point, no alternatives discussion) and a 02→04 secrets inventory plus never-log list — the decisions exist in `api-auth-design.md` / `connector-security.md` but not yet in that form. 46 47 Coordinated with `../05-data-ops/CLAUDE.md` on `user_credential.method` being modeled as a lookup table (not a native enum) — the credential-storage design in `threat-model.md` is method-aware as a result (password vs. WebAuthn vs. TOTP each have different storage requirements), not a single password-hashing assumption. 48 49 Phase 1 (org planning): `PLAN.md` in this directory is the track plan — eleven Stories, a four-hire roster (Adversary, Principled Architect, Tinkerer, Designated Skeptic), the debate plan, and dependencies; `../CASTING.md` §5 ratified with four ⟨02⟩ amendments; `planning-approach.md` gained Appendix A (attack-tree template) and Appendix B (harness constraints). Awaiting team-lead's "hiring go" before any persona or agent definition is created. 50 51 Next: available for `03-engineering-delivery` to implement against; open to review/adjustment as data-ops' schema and engineering's implementation surface any gaps. 52 53 <!-- profile-gen:start slug=marcus-ilori --> 54 @profiles/marcus-ilori/marcus-ilori.md 55 <!-- profile-gen:end slug=marcus-ilori --> 56
{
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/PLAN.md"
}1 # Track 02 Plan — AI Architecture & Security Architecture (Phase 1)
2
3 Owner: Marcus Ilori. Written in plan mode per `PLAN.md` Phase 1. Once approved it is copied verbatim to `02-ai-security-architecture/PLAN.md`; the `~/.claude/plans/` copy is scratch.
4
5 ## Context
6
7 The org plan (`PLAN.md`) requires every lead to plan in plan mode with four mandatory sections before hiring. Track 02 carries two extra obligations as the AI-architecture owner: ratify or amend the orchestration-pattern catalog in `CASTING.md` §5, and check the harness facts in `PLAN.md` "Verified mechanics" against `research-claude-architecture-best-practices.md` before other leads build on them. The four first-wave deliverables of this track (`threat-model.md`, `api-auth-design.md`, `connector-security.md`, `ai-workflow-narrative.md`) are complete; this plan turns them into debated, second-reviewed, receiver-shaped outputs and adds the org-layer story to the AI narrative. No code anywhere in this plan.
8
9 ## 0. Ratification of `CASTING.md` §5 and the mechanics check
10
11 **Verdict on §5: ratified, with four amendments.** The six protocols are sound and their use/waste assignments match my own planning-approach reasoning. The amendments each target a structural weakness that the palette's own blind-spot column predicts:
12
13 1. **Red team / blue team** — add a mandatory ranking step: the Adversary hands over a *top-five* ranked list of leaves before the blue seat responds (counters "sees threats everywhere, no prioritization" structurally rather than by exhortation). Add an optional one-turn *alternatives* seat for the Tinkerer between red and blue: propose design changes that delete leaves outright — a leaf that cannot exist beats a control. Add a residual-risk scoring rule (below). Point to the attack-tree template in `planning-approach.md` Appendix A.
14 2. **Adversarial pair** — every numbered objection from the Designated Skeptic must end with a one-line "what would change my mind" (counters "objections without a counter-proposal"). The Proposer must answer every objection in writing; it may decline to revise, but not silently.
15 3. **Structured written debate** — the lead's synthesis must state which position was stronger on which point and why; preserving both positions is necessary but not a substitute for a ruling (counters the Consensus Weaver blind spot, which is the PM's own listed archetype).
16 4. **All patterns** — each decision artifact records, in a footer, the model each seat ran on and the number of hire turns consumed, so `ai-workflow-narrative.md` can cite measured cost rather than intent.
17
18 Where the text lives, one source of truth per thing: the four protocol amendments go into `CASTING.md` §5 in place (my permitted scope); the attack-tree *template* — a table shape, not a protocol — goes into `planning-approach.md` as **Appendix A** with a one-line pointer from §5. No new document.
19
20 **Attack-tree template (Appendix A content):** columns `# | Leaf (attack path toward the goal) | Precondition (what the attacker must already have) | Likelihood L/M/H | Impact L/M/H | Blue response: Control or Accepted risk | Owner (02 requirement / 03 implements / 04 mechanism) | Residual L/M/H | Source`. Rules: red ranks the top five first; blue answers every leaf, and an "accepted risk" must say why it is acceptable *for this system*, not in general; the lead assigns Residual with a one-line justification; any Residual **H** must produce either a follow-up Story or an accepted risk visible to Warren in the artifact. The completed table is appended to the surface's design doc (`connector-security.md`); the full positions live in `decisions/<topic>.md`.
21
22 **Mechanics check against the research doc: no contradictions.** Three notes for the record, to be added as a short "Harness constraints" addendum to `planning-approach.md` so it stays the single source of truth for the tiers:
23
24 - Effort: the harness has no per-agent effort key; teammates inherit `high`, and `modelSettings.haiku.effortLevel: low` is the only per-model lever. My sonnet-medium tier therefore collapses to sonnet-high (accepted — errs upward on the tracks that matter), and *sonnet/low research fan-out is unavailable*: research is haiku/low, or sonnet/high when judgment is needed, which is costlier than planned and should be used sparingly.
25 - The per-agent `PreToolUse` hook and the `.claude/agents` frontmatter facts are outside my research doc's scope; I have no primary-source basis to dispute them and record them as verified by team-lead, not by 02.
26 - Research doc §4 (prompt caching) still applies one level down: a hire's `.claude/agents/<slug>.md` is part of that hire's stable prefix. Do not edit a hire's definition while it is running; edit at spawn boundaries.
27
28 ## 1. Deliverables and tasks (Epic: *02 AI & Security Architecture*; one Story each)
29
30 | # | Story | Hand-off produced (file) | Consumer | Depends on |
31 |---|---|---|---|---|
32 | S1 | Ratify/amend `CASTING.md` §5 (four amendments above) and add Appendix A to `planning-approach.md` | `CASTING.md` §5; `planning-approach.md` App. A | all leads, before any red/blue runs | — |
33 | S2 | "Harness constraints" addendum to `planning-approach.md` | `planning-approach.md` | all leads; `ai-workflow-narrative.md` | — |
34 | S3 | Red/blue on the connector token lifecycle → attack-tree table with residual scores | `connector-security.md` (table appended); `decisions/connector-token-lifecycle-redblue.md` (full record) | 03's opus token-lifecycle seat; 04 (any control that is a mechanism) | hires; S1 |
35 | S4 | Adversarial pair on search-API authorization scoping | `decisions/search-authz-scoping.md`; `api-auth-design.md` gains "Objections considered" | 03 (middleware) | hires |
36 | S5 | Structured written debate on the core claim of `ai-workflow-narrative.md` | `decisions/ai-workflow-claim-debate.md` | S9 | hires |
37 | S6 | **02→03 receiver-shaped auth hand-off** — token scheme (client-credentials → JWT, asymmetric signing, claim set `sub/scope/exp/iat/aud`, TTL 5–15 min, no refresh, no denylist), **where validated** (middleware at the API edge; key retrieval), the final scope vocabulary and object-level policy hook from S4, pagination ceiling and per-client rate limit as requirements, audit-log field list, and an explicit "not yours to decide" list | `handoff-03-auth.md` | 03 — Renata scaffolds now, needs this before middleware is finalized | S4 |
38 | S7 | **02→04 secrets inventory + never-log list** — table `secret | used by (API / connector / authz server) | injection requirement | rotation expectation | never-log`; never-log list (vendor password, vendor token, PII field values, raw vendor error bodies, JWT signing private key, client secrets). Assumes **Kubernetes-native Secret delivery on the LAN lab cluster** 04 names as the design-only target (Warren's decision via Theo, `LOG.md`), stated explicitly rather than a generic "managed store"; requirements still hold if the mechanism changes | `handoff-04-secrets.md` | 04 — gates Theo's producer phase | one Skeptic objection turn; no debate |
39 | S8 | Independent second-review pass on the security-graded docs (`threat-model.md`, `api-auth-design.md`, `connector-security.md`) by hires who did not author or blue-team them | `decisions/second-review-security-docs.md`; fixes applied in place | Warren / final consistency pass | S3, S4 |
40 | S9 | Revise `ai-workflow-narrative.md` to cover the org layer — personas, patterns, debates, measured turn counts and models from the decision-artifact footers; incorporate the S5 ruling honestly, whichever way it goes; state plainly that a real external agent (the cluster operator 04 describes) exists in the environment and was deliberately kept out of scope — never contacted, nothing deployed | `ai-workflow-narrative.md` | submission reader | S3–S5 |
41 | S10 | `threat-model.md` maintenance: (a) repoint the stale reference to `05-data-ops/schema/postgres_cockroachdb.sql` (wiped) at the prose schema in `../05-data-ops/multi-db-strategy.md` — verified 2026-09-12 that `auth_method` and `secret / hash_algo / hash_cost` are already there in prose; (b) add one trust-boundary line: an **external operator agent applying manifests to the deployment target** (04's design-only lab cluster) is a privileged actor outside this service — whoever can apply a manifest can read any Secret it mounts — so the assumption "secrets are safe once in the cluster" is named, not silent | `threat-model.md` | 04 | none — can run at Phase 3 start |
42 | S11 | Post-implementation independent review of 03's auth middleware and connector token code (security-graded surfaces only) | `decisions/second-review-implementation.md` | 03 | Warren's code go-ahead — Phase 4+, listed now so it is not forgotten |
43 | T1 (Task) | Deep research, pre-authorized for one topic only if the Adversary or Architect requests it: bearer-token handling and OAuth 2.0 security best current practice (RFC 6750, RFC 9700) to arm S3 with primary sources; haiku/low fan-out run by me | `research-oauth-token-handling.md` | S3 | request from a hire |
44
45 Cross-team asks (not my stories): 04's Newcomer performs the cold read of `handoff-03-auth.md` and `handoff-04-secrets.md` (already the one pattern `CASTING.md` assigns 04); the red/blue on the connector is run **once**, here, with 03's opus seat reading the artifact rather than 03 running a second one — I will flag this to Renata so it is not duplicated.
46
47 ## 2. Hire roster (4; all sonnet; all read-only per `_TEMPLATE.md` — they return prose, I write the artifacts)
48
49 No first name collides with Dana, Naomi, Marcus, Renata, Theo, or Priya. Roster spans serious↔playful and cautious↔bold; two hires are `playful, bold`, contributing to the org-wide minimum of three.
50
51 ### H1 — Tomasz Wrede · Adversary · `playful, bold` · function: attack-tree author
52 Patterns: red/blue (red seat, S3); optional one-turn pre-read on S4 ("what would I do with each scope"). Requester for T1.
53
54 > Mischievous and quick — the reviewer who reads a sequence diagram and immediately asks where he would stand to steal from it, and grins while asking. Decides by enumerating attack paths first and only then asking which ones matter, which he admits is backwards. Pushes back hardest on any claim that a control is "sufficient" until someone names the attacker it stops and the one it does not. Eight years on an internal red team at a large retailer, then four running penetration tests against payment and identity APIs for a consultancy; fluent in token theft, replay, and cache-poisoning paths, less so in formal proofs. Blind spot: he sees threats everywhere and ranks them poorly — left alone he produces forty leaves of equal weight, so make him hand over a top five before anyone answers. Argues cheerfully, concedes fast when a leaf is shown unreachable, and keeps a list of the ones he lost.
55
56 ### H2 — Helena Marsh · Principled Architect · `serious, cautious` · function: controls author and standards reviewer
57 Patterns: red/blue (blue seat, S3); adversarial pair (Proposer, S4); second review of `threat-model.md` and `api-auth-design.md` (S8 — she did not author them and is not blue on them).
58
59 > Measured and exact; in a meeting she speaks last, usually with a section number. Decides from the threat model downward and writes every decision as a record with the alternatives she rejected and why. Pushes back on any control offered without the attack it defeats, on any trade of a security property for developer convenience, and on a citation that points at a blog when a specification exists. Eleven years across a national bank's identity platform and a government PKI programme; deep in OAuth 2.0 and its security best current practice, JOSE and the ways JWT libraries fail, and TLS deployment. Blind spot: over-specification — she designs for threats this take-home does not have and can mistake thoroughness for finished, so pair her with someone asking what the assignment actually needs. Courteous under challenge, unhurried, and changes position when shown evidence, saying so plainly.
60
61 ### H3 — Felix Adebayo · Tinkerer (silly & innovative) · `playful, bold` · function: alternatives generator — **my temperament opposite (required)**
62 Patterns: red/blue (alternatives seat between red and blue, S3 — "what if the connector held no token at all?"); structured written debate (Position A, S5).
63
64 > Playful and fast-associating; he describes a bearer token as a hotel keycard that opens every door until Thursday, and means it as a design question. Decides by sketching three alternatives, one deliberately strange, before defending any, and drops his own without ceremony when someone shows a cleaner failure mode. Pushes back on "that is the standard approach" whenever nobody in the room can say what the standard protects against, and on any diagram with a box labelled "just cache it". Five years building fraud-scoring pipelines at a payments startup, three at a mid-sized identity vendor prototyping passkey flows; Go and Rust, and an unreasonable fondness for property-based tests. Blind spot: novelty bias — he underweights boring, proven controls and needs someone to cite the RFC at him. Warm in disagreement, never sarcastic, delighted to be wrong in an interesting way.
65
66 ### H4 — Ingrid Solano · Designated Skeptic · `serious, bold` · function: standing objector and independent reviewer
67 Patterns: adversarial pair (Skeptic, S4); structured written debate (Position B, S5); one objection turn on the secrets inventory (S7); second review of `connector-security.md` (S8 — she is not blue on it).
68
69 > Dry and direct; in a meeting she is the one asking "compared to what?" before the proposal is finished. Decides only after hearing the strongest case against the room's favourite, and makes that case herself if nobody else will, even when she privately agrees. Pushes back on consensus that arrived too quickly, on scope vocabularies invented before anyone listed the callers, and on evidence that is one number from one vendor. Nine years as a security reviewer on an enterprise SaaS platform, then three chairing design reviews at a healthcare data company where every authorization decision met a regulator. Background in authorization models — RBAC, ABAC, object-level policy — and in reading audit logs after something went wrong. Blind spot: objections without a counter-proposal — she can leave a team knowing what is wrong and not what to do, so every objection carries a "what would change my mind" line. Polite, unyielding on process, quick to record when an objection was answered.
70
71 Registry rows (appended to `CASTING.md` §8 only after "hiring go"): `tomasz-wrede`, `helena-marsh`, `felix-adebayo`, `ingrid-solano`; Track 02; Model sonnet; Persona `02-ai-security-architecture/profiles/<slug>/<slug>.md`; Agent def `.claude/agents/<slug>.md`; Portrait `queued`.
72
73 ## 3. Debate plan
74
75 | Decision | Pattern | Seats | Turns | Artifact |
76 |---|---|---|---|---|
77 | Connector token lifecycle: TTL, at-rest encryption, per-vendor keying, fail-closed, "hold no token" alternative | Red/blue (amended) | Tomasz red → Felix alternatives → Helena blue → Tomasz one rebuttal on the top three → Marcus scores residual | 4 + lead | `decisions/connector-token-lifecycle-redblue.md`; table into `connector-security.md` |
78 | Search-API authz scoping: `profile:search` vs `profile:read:own` vs `profile:read:any`; object-level policy; who gets `profile:search` at all | Adversarial pair (amended) | Helena proposes → Ingrid numbered objections with "what would change my mind" → Helena answers/revises → Marcus rules (optional Tomasz pre-read, 1 turn) | 3 (+1) + lead | `decisions/search-authz-scoping.md`; "Objections considered" into `api-auth-design.md`; feeds S6 |
79 | Core claim of `ai-workflow-narrative.md` — does the multi-agent, persona-driven workflow produce *better design* than one strong session, or is it cost without evidence? (Anthropic's 90.2% figure is for research breadth, not design quality) | Structured written debate (amended) | Felix: it does, and the narrative undersells it → Ingrid: unproven; keep the narrow claim → Marcus rules per point, and the ruling goes into the narrative whichever way it lands | 3 + lead | `decisions/ai-workflow-claim-debate.md` |
80 | Secrets inventory completeness | Single objection turn | Ingrid: what is missing / mis-owned | 1 | inline in `handoff-04-secrets.md` |
81 | Security-graded docs, independent read | Second review | Helena on threat model + API auth; Ingrid on connector security | 2 | `decisions/second-review-security-docs.md` |
82
83 Budget: about 14 hire turns for the whole track, all sonnet/high. **Declared waste — not debated:** the Argon2id hashing decision (settled by OWASP/NIST — cite, do not debate); the OAuth2 grant *type* (already reasoned in `api-auth-design.md`; S4 is about scoping, not the grant); STRIDE as the framework; endpoint paths and JSON bodies (fixed by the assignment); anything about DAO CRUD or Dockerfiles; a three-hats run (no decision in this track has three legitimate optimist/pessimist/pragmatist positions). No pattern gets a second run "to be sure".
84
85 ## 4. Dependencies
86
87 **From 05 (Priya):** the `auth_method` lookup-table shape (`id, name, requires_secret, is_active`; `user_credential.secret / hash_algo / hash_cost`) — **settled** by direct two-party coordination and confirmed present in prose in `multi-db-strategy.md`; my threat model is already method-aware. Nothing further needed from 05 for this track's Phase 3; the Go-shaped contract she is delivering is 03's dependency, not mine.
88
89 **From 01 (Naomi):** `api-connector-design-rationale.md` as the single source of truth for field/endpoint shape — a soft dependency, read before finalizing S6, not before starting.
90
91 **To 03 (Renata):** `handoff-03-auth.md` (S6) — she scaffolds now and needs it before middleware finalization; target: immediately after S4 rules, early Phase 3. The red/blue table (S3) is the requirement set for her opus token-lifecycle seat; target mid Phase 3. One red/blue, run here, read there.
92
93 **To 04 (Theo):** `handoff-04-secrets.md` (S7) — gates his producer phase; it depends on no debate, only one objection turn, so it is the **first** deliverable out after hiring. His Newcomer cold-reads both hand-offs.
94
95 **From 04 / Warren (via team-lead, recorded in `LOG.md`):** the deployment target is a real Kubernetes lab cluster on the LAN operated by an external agent, used as a **design-only** reference. This track assumes Kubernetes-native Secret delivery in S7 and names the operator as a trust boundary in S10. Hard rule inherited: no action on that cluster and no contact with its operator agent, ever, without Warren's explicit go — this track has no reason to touch either.
96
97 **To all leads:** `CASTING.md` §5 amendments and Appendix A (S1) before any lead runs a red/blue; the harness-constraints addendum (S2) before anyone spawns a "sonnet/low" research subagent that will actually run at high.
98
99 **From team-lead:** "hiring go" — nothing in §2 is created or spawned before it.
100
101 ## Files touched after ExitPlanMode (Phase 1 close-out only)
102
103 1. Copy this plan → `02-ai-security-architecture/PLAN.md`.
104 2. `PLANNING.md` — my row only → `plan drafted — awaiting hiring go`, hires `4 / 0 / 0`, planning notes pointer, date.
105 3. `CASTING.md` §5 — the four amendments in place, nothing outside §5.
106 4. `planning-approach.md` — Appendix A (attack-tree template) and the Harness-constraints addendum.
107 5. `02-ai-security-architecture/CLAUDE.md` Status — one line pointing at `PLAN.md` (turn-boundary edit).
108 6. ≤200-word summary with roster to team-lead.
109
110 Not touched until "hiring go": `profiles/`, `.claude/agents/`, `
… truncated (20 KB cap){
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/profiles/ingrid-solano/ingrid-solano.md"
}1 --- 2 schema_version: 1 3 name: "Ingrid Solano" 4 slug: "ingrid-solano" 5 image: "profiles/ingrid-solano/ingrid-solano.png" 6 7 nsfw: false 8 display: 9 image: true 10 name: true 11 autostart: false 12 generation: 13 backend: "comfyui" 14 model: "comfyui-workflow.json" 15 prompt: "Ingrid Solano, a woman in her early 40s with Latin American features, dark hair in a low bun, a level direct gaze with one eyebrow slightly raised, wearing a dark green blouse under a black blazer, head-and-shoulders portrait, centered, neutral dark gray studio background, professional corporate headshot photography, soft studio lighting, sharp focus, photorealistic" 16 negative_prompt: blurry, low quality, distorted, extra limbs, watermark, text, nsfw, nude, nudity, explicit, sexual content 17 seed: null 18 gif_mode: null 19 created_at: "2026-09-12T00:00:00Z" 20 --- 21 22 # Ingrid Solano 23 24  25 26 27 ## Personality 28 29 Dry and direct; in a meeting she is the one asking "compared to what?" before a proposal is finished. Decides only after hearing the strongest case against the room's favourite, and makes that case herself if nobody else will. Pushes back on consensus that arrived too quickly, on scope vocabularies invented before anyone listed the callers, and on evidence that is a single vendor's number. Nine years as a security reviewer at a SaaS vendor, then three chairing design reviews at a healthcare data company where every authorization decision met a regulator. Background in authorization models — RBAC, ABAC, object-level policy — and in post-incident audit-log review. Blind spot: objections without a counter-proposal — she can leave a team knowing what is wrong and not what to do, so every objection carries a "what would change my mind" line. Polite, unyielding on process; records when an objection was answered. 30 31
{
"command": "python3 /Users/warrenthompson/.claude/skills/profile-gen/scripts/show_profile.py --profile /Users/warrenthompson/Source/LoginID/02-ai-security-architecture/profiles/ingrid-solano/ingrid-solano.md --root /Users/warrenthompson/Source/LoginID/02-ai-security-architecture"
}{"name": "Ingrid Solano", "mode": "hud", "hud": true, "badge": false, "background": false, "inline": false}{
"to": "marcus-ilori",
"content": "Ingrid here — oriented on root CLAUDE.md, track CLAUDE.md, PLAN.md, and my persona. Ready for S4 (Skeptic seat), S5 (Position B), S7 (secrets-inventory objection), S8 (second review of connector-security.md). Holding until called."
}<tool_use_error>Error: No such tool available: SendMessage. SendMessage is disabled for this session, in subagents as well as here.</tool_use_error>
{
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/handoff-04-secrets.md"
}1 # Hand-off 02 → 04: Secrets inventory and never-log list 2 3 Status: **DRAFT v0** — written by Marcus Ilori before the Designated Skeptic's objection turn (S7 in `PLAN.md`); becomes v1 after Ingrid Solano's objections are answered and 04's Newcomer (`wesley-okonkwo`) has cold-read it. Receiver: Theo Bergman, `../04-infra-devops/`. Written to the project's hand-off standard: 04 should be able to act on this without re-deriving the reasoning in `threat-model.md`, `api-auth-design.md`, or `connector-security.md`. 4 5 ## Assumptions this hand-off makes 6 7 1. Deployment target is the **design-only Kubernetes lab cluster** 04 names (Warren's decision via Theo, `../LOG.md`). Delivery mechanism is therefore Kubernetes-native `Secret` objects unless 04 chooses an external store; every requirement below is written so it still holds if the mechanism changes. 8 2. Service topology is 03's published boundary (`../PLANNING.md`, Service boundaries): two binaries, `cmd/api-service` (Q1 DAO + Q2 REST API) and `cmd/idp-connector` (Q3), env-var configuration only. Variable names below are 03's; two are marked as pending this track's final scheme. 9 3. An **external operator agent applies manifests** to the cluster. That operator is a privileged actor outside this service: anyone who can apply a manifest can read any Secret it mounts. Consequence for 04: manifests reference Secrets by name and never carry values; values are set out-of-band. This is a named trust boundary in `threat-model.md`, not a silent assumption. 10 4. Nothing is deployed and nobody contacts the cluster or its operator during the take-home. This document describes what *would* be delivered, not what has been. 11 12 ## Secrets inventory 13 14 | # | Secret | Used by | Injection requirement (02 sets it) | Rotation expectation | Kubernetes-native note (04's mechanism) | Never-log | 15 |---|---|---|---|---|---|---| 16 | 1 | Database credentials, carried inside `DB_DSN` | `api-service` | Runtime-injected; never in source, image, or committed config | On credential rotation; service restart or reload acceptable | `Secret` → env var is acceptable here only because 03 chose env-var config; prefer a file mount if 03's config loader allows it — env vars surface in `kubectl describe`, crash dumps, and child processes | Yes — the DSN contains the password; log the driver name and host at most | 17 | 2 | JWT signing **private** key (RS256/ES256), per `api-auth-design.md` | Token-issuing authorization-server component only — **not** `api-service`, which verifies with the public key | Runtime-injected as a **file**, never an env var; asymmetric so the API never holds it | Key rotation with a `kid` claim; publish old and new public keys together during overlap; TTL of tokens (5–15 min) bounds the overlap window | `Secret` mounted read-only as a file; restrict `get`/`list` on this Secret to the issuer's service account via RBAC | Yes | 18 | 3 | JWT **public** key / JWKS | `api-service` middleware | Not a secret; may ship in config or be fetched from the issuer | Follows #2 | Plain `ConfigMap` or fetched URL; listed here so 04 does not over-protect it | No (but never log full tokens — see below) | 19 | 4 | API caller `client_secret`s (OAuth2 client-credentials) | Authorization server (verifies); callers hold their own copy | **Stored hashed** (Argon2id, same floor as `threat-model.md` Asset 1), never plaintext at rest; issued once at provisioning and not recoverable | Per-client rotation on demand; revoke by deleting the hash | Lives in the issuer's datastore, not a Kubernetes `Secret`; 04 delivers only that datastore's credentials (#1-shaped) | Yes | 20 | 5 | Connector's own vendor registration: `IDP_ABC_CLIENT_ID`, `IDP_ABC_CLIENT_SECRET` (and the XYZ pair if a second vendor is configured) | `idp-connector` | Runtime-injected; never in source or committed config; `client_id` is low-sensitivity but travels with the secret and gets the same handling | Per vendor's policy; expect manual rotation | One `Secret` per vendor so rotating ABC never touches XYZ; file mount preferred | Yes for the secret; keep the id out of logs too | 21 | 6 | Vendor `access_token` (obtained via `/auth`, used for `/identity`) | `idp-connector`, in memory | **Not a deployed secret** — transient. If cached at all: TTL ≤ vendor expiry, encrypted at rest with #7, keyed per vendor account (`connector-security.md` §1) | Expires on its own; never persisted across restarts | No `Secret` object; if 04 provisions a cache, it must be encrypted using #7 and reachable only from `idp-connector` | Yes — never, including error paths | 22 | 7 | Data-encryption key (KEK) for envelope encryption of any TOTP shared secrets and any cached vendor tokens | `api-service` (TOTP) and `idp-connector` (token cache), if either feature exists | Runtime-injected as a file; the KEK never leaves the process that decrypts | Rotation by re-wrapping DEKs; never requires re-hashing passwords (those are hashed, not encrypted) | `Secret` (file mount) or an external KMS if 04 prefers — the requirement is only that a KEK exists and is not in source | Yes | 23 | 8 | TLS serving private keys for `api-service` and `idp-connector` | Ingress / each binary, per 04's termination choice | Runtime-injected; TLS 1.2+ only; certificate validation never disabled in any environment (`connector-security.md` §2) | Per certificate lifetime; automated issuance preferred | `kubernetes.io/tls` `Secret`; issuance mechanism is 04's call | Yes | 24 | 9 | End-user's vendor password (`POST /auth` request body) | `idp-connector`, transient | Never persisted, never cached, held only for the vendor call | n/a | Nothing for 04 to deliver; listed so nobody creates a place for it | **Yes — under no circumstance, including stack traces** | 25 26 Names marked pending in 03's boundary: `AUTH_JWT_ISSUER` / `AUTH_JWT_AUDIENCE` are configuration, not secrets; this track will confirm their final values in `handoff-03-auth.md`. They are listed here only so 04 does not classify them as secrets. 27 28 ## Kubernetes-specific requirements (04 implements; 02 sets) 29 30 - **Secrets at rest in etcd are base64, not encrypted, by default.** Either enable encryption at rest for the `Secret` resource type, or source values from an external store that never writes plaintext to etcd. One or the other; the design must say which. 31 - **RBAC:** no service account may `get`/`list` Secrets it does not mount. The operator agent's apply rights are a documented trust boundary, not a hole to close in this take-home — but the design should note that its scope could be narrowed to a namespace. 32 - **Manifests carry references, never values.** Any manifest in the repository contains Secret *names*; values are provided out-of-band. This is what keeps assumption 3 honest. 33 - **Prefer file mounts over env vars** for #2, #5, #7, #8. #1 is env-var only because 03's config surface is env-var only; if 03 accepts a `_FILE` suffix convention, move it. 34 - **No secret in an image layer, build arg, or CI log.** CI redaction is 04's mechanism; the requirement is that a build log can be published without exposing anything in this table. 35 36 ## Never-log list (applies to every component, every environment, every log level, including error paths and stack traces) 37 38 1. End-user vendor password (#9). 39 2. Vendor `access_token` (#6) — and any `Authorization` header value, inbound or outbound, in full. 40 3. PII field values from `user_profile` or from `/identity` responses: name, phone, `street_address`, `locality`, `region`, `postal_code`, `country`. Log the *fact* of access — caller `sub`, scope used, opaque record id, timestamp, status — never the values. 41 4. Raw vendor error response bodies (they can echo submitted PII or partial credentials). Log status code and vendor error code only. 42 5. JWT signing private key (#2) and any JWT in full — log `sub`, `aud`, `exp`, and `kid` if needed, never the token. 43 6. API caller `client_secret`s (#4), in plaintext or hashed. 44 7. `DB_DSN` (#1) or any connection string containing credentials. 45 8. Full request bodies of `POST /auth` and `POST /identity` — both contain items above by construction. 46 9. Search query parameters containing a name or phone fragment (`api-auth-design.md`, hardening) — they are PII; redact or hash before any access log, including proxy and ingress logs 04 controls. 47 48 ## What 04 does not decide (already decided upstream) 49 50 Token TTLs and claim set (`api-auth-design.md`); scope vocabulary (`handoff-03-auth.md`, pending); password and client-secret hashing (`threat-model.md` Asset 1); whether the connector caches vendor tokens (`connector-security.md` §1 — it may, under the conditions in #6); PII retention windows (`../05-data-ops/pii-governance.md`). 51 52 ## Open items for the objection turn 53 54 - Is anything in this table mis-owned between 02 (requirement), 03 (implements), and 04 (mechanism)? 55 - Is a secret missing? Candidates I considered and excluded: session cookies (none — machine-to-machine API), SMTP/notification credentials (no such feature in scope), object-storage keys (no such component). 56 - Is the env-var exception for #1 acceptable, or should 02 push 03 for a `_FILE` convention now? 57
{
"command": "true"
}(Bash completed with no output)
{
"command": "python3 /Users/warrenthompson/.claude/skills/profile-gen/scripts/show_profile.py --profile ./profiles/ingrid-solano/ingrid-solano.md --root ."
}{"name": "Ingrid Solano", "mode": "hud", "hud": true, "badge": false, "background": false, "inline": false}{
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/ai-workflow-narrative.md"
}1 # AI Tooling & Workflow Narrative
2
3 Owner: Marcus Ilori (`02-ai-security-architecture`). This is the full answer to LoginID's explicit ask ("describe any tool, framework, or AI used") — other tracks note their own AI usage briefly and point here for depth, per `../CLAUDE.md`.
4
5 This document states what was done, distinguishes what's independently documented by Anthropic from what this project composed on its own, and is deliberately precise about that boundary rather than overclaiming an official pattern that doesn't exist. Full source-by-source detail lives in `research-claude-architecture-best-practices.md`; this document is the applied summary a reviewer can read without the research doc.
6
7 ## Tool
8
9 **Claude Code**, Anthropic's agentic CLI. Chosen because the deliverable itself is an exercise in demonstrating AI-architect judgment — using the tool that is itself the subject of the "how did you use AI" question, rather than a general-purpose chat interface, lets the workflow be inspected as part of the submission.
10
11 ## Structure: a root brief plus five track-scoped briefs
12
13 The repository is organized as one root `CLAUDE.md` (the assignment text, track boundaries, cross-track working principles) plus five track directories, each with its own `CLAUDE.md` scoped to a distinct slice of the problem (product/industry framing, this track's security architecture, engineering delivery, infra/devops, data ops). Each track file is written to be self-contained enough that a fresh agent with no other conversation history can pick it up correctly given only that file plus the root file.
14
15 **What this composes, precisely:**
16
17 1. **The `CLAUDE.md` mechanism itself** is Anthropic's own, documented at https://code.claude.com/docs/en/memory — auto-loaded at session start, meant to be treated as living documentation, kept short and specific.
18 2. **The decision to fan that mechanism out per-directory, with a stable root file for cross-cutting state**, is this project's own application of a general principle Anthropic documents separately in "Effective context engineering for AI agents" (https://www.anthropic.com/engineering/effective-context-engineering-for-ai-agents): structured notes persisted to files outside the live context window, retrieved just-in-time rather than all loaded upfront, so a track's subagent carries only the ~1-2 files it actually needs rather than the whole project's accumulated context.
19
20 I'm stating this distinction explicitly because it matters to how this narrative should be read: Anthropic has not published a named "root CLAUDE.md + per-directory CLAUDE.md" pattern, and claiming they had would be exactly the kind of unsupported assertion this track's whole discipline argues against. What's true and defensible is narrower and still worth the credit — this is a reasonable, motivated composition of two things Anthropic documented separately.
21
22 ## Orchestration: scoped subagents per track, not one long single-context session
23
24 Each track is intended to be worked by an agent (in practice here, a persona-bearing teammate agent per track — see each track's `profiles/` entry) that reads the root file plus its own track file and works within that scope, rather than one agent holding all five tracks' context simultaneously and switching between them. This follows the orchestrator-worker pattern Anthropic describes in "How we built our multi-agent research system" (https://www.anthropic.com/engineering/multi-agent-research-system): a coordinating layer decomposes work into distinct-objective, distinct-tool-scope units; each unit works its own trajectory; only condensed results cross back, not raw exploration. Anthropic's own internal figure for a subagent-to-coordinator handoff is roughly 1,000-2,000 tokens of distilled findings — the discipline this project applies at track boundaries is the same: a track's `CLAUDE.md` plus its finished deliverable files (this document included) are what another track reads, never a transcript of how those conclusions were reached.
25
26 Concretely for this track: the threat model, the API auth/authz design, and the connector security design (`threat-model.md`, `api-auth-design.md`, `connector-security.md`) are the condensed artifacts `03-engineering-delivery` needs to implement against. It should read those files, not need this track's reasoning replayed turn-by-turn.
27
28 ## What was AI-generated vs. human-directed
29
30 - **Human-directed**: the assignment itself (LoginID's), the five-track decomposition and ownership boundaries (the root `CLAUDE.md`), and every judgment call in this document and its siblings about *which* security mechanism to choose and why (e.g., OAuth2 client-credentials over static API keys in `api-auth-design.md` — a reasoned choice, not a default).
31 - **AI-generated, human-directed content**: the research pass that produced `research-claude-architecture-best-practices.md`, and the drafting of this narrative, the threat model, and the design documents themselves, all produced within this track's scope by an agent operating under this track's `CLAUDE.md` and persona brief.
32 - **Explicitly not claimed**: that any of this constitutes a validated, "certified" methodology. The research doc's §6 covers Anthropic's Claude Certification Program by name and is careful that its existence is not evidence this project's specific techniques are exam-validated — that would be a citation used to imply authority it doesn't carry.
33
34 ## Why this discipline matters for this specific track
35
36 This track's whole mandate is "name assumptions, cite sources, treat security design as a first-class deliverable." Applying the identical standard to the AI-workflow claims — say precisely what's sourced, say precisely what's this project's own extension, don't blur the two — is the same rigor applied to a different subject. A reviewer evaluating "how you use AI tooling" should be able to trust this document's citations exactly as much as they can trust the threat model's.
37
38 ## Sources
39
40 Full list with URLs in `research-claude-architecture-best-practices.md`. Load-bearing ones for this document specifically:
41
42 - https://code.claude.com/docs/en/memory
43 - https://www.anthropic.com/engineering/effective-context-engineering-for-ai-agents
44 - https://www.anthropic.com/engineering/multi-agent-research-system
45 - https://claude.com/blog/subagents-in-claude-code
46 {
"file_path": "/Users/warrenthompson/Source/LoginID/02-ai-security-architecture/connector-security.md"
}1 # IDP Connector Security Design — Question 3
2
3 Owner: Marcus Ilori (`02-ai-security-architecture`). Covers the third-party identity-provider connector (generic vendors "ABC"/"XYZ") exposing `/auth` and `/identity`, per the assignment. Implementation target: `../03-engineering-delivery/CLAUDE.md`.
4
5 ## Contract recap (from the assignment, verbatim in `../CLAUDE.md`)
6
7 - `POST /auth` — body `{"username": "<string>", "password": "<string>"}` → returns an `access_token`.
8 - `POST /identity` — body `{"phone": "<string>", "name": "<string>"}` → returns PII (name, phone, address fields).
9
10 Read literally, this connector is a thin proxy in front of ABC/XYZ: it takes vendor credentials, gets a vendor token, and uses that token to pull PII from the vendor. Every security question here is "what does this service do with a vendor secret and vendor PII while it's in our custody," since we don't control the vendor's own security posture.
11
12 ## Threat surface specific to this connector
13
14 This extends the third-party-token row in `threat-model.md`; this document is the concrete design against those threats.
15
16 ### 1. What happens to the vendor `access_token` once retrieved
17
18 - **Never logged.** The `/auth` request body contains a vendor password; the response contains a bearer token. A naive "log the request and response for debugging" habit puts both in the same log line — this is called out explicitly because it's the single most common way a connector like this leaks a credential in practice, not a hypothetical.
19 - **No persistent or cached token, by default.** Revised by the S3 red/blue run (`decisions/connector-token-lifecycle-redblue.md`): the token is fetched fresh from `/auth` for each `/identity` call, held only inside that single request's execution scope, and discarded when the call returns — not written to any cache or datastore as a matter of course. Caching is an explicitly-justified exception requiring its own threat re-review if latency or vendor rate limits ever force the question; if that exception is ever opened, it must satisfy: (a) cache key derived from an immutable, vendor-issued identifier (our tenant id + vendor id + the vendor's own opaque subject id) — never from request-supplied `name`/`phone`, which are attacker-influenced; (b) TTL no longer than the vendor's own token expiry, encrypted at rest (envelope encryption via KMS, mechanics owned by `../04-infra-devops/CLAUDE.md`); (c) if the KMS/envelope key is unreachable at write or read time, fail closed to "no cache, fetch live" for that call — never to an unencrypted write and never to a hard connector failure.
20 - **Fetch-use-zeroize boundary, stated explicitly so it survives implementation:** the token variable's scope runs from the `/auth` response to the immediately following `/identity` call and no further; it is not passed up the call stack beyond the vendor-call function, and it is not reused across a retry. **A retried `/identity` call re-fetches via `/auth`; it never reuses a token held from a prior attempt** — otherwise the "one request" exposure window `§4`'s retry/backoff allowance quietly becomes "however long the retry policy runs." Go gives no guaranteed memory zeroing without unsafe/cgo, so a crash dump taken mid-call could still capture the token in the vendor-call function's stack; that residual is accepted given the window is now one function call, not a cache TTL (core-dump suppression at the runtime level is a `../04-infra-devops/CLAUDE.md` mechanism, requirement recorded here, not owned here). The token field is excluded from any generic request/response dump used in error handling — the same log-hygiene discipline below applies to panic traces and debug dumps, not just application logs.
21 - **The outbound HTTP client to the vendor must construct a fresh, isolated `Authorization` header per request, never inherited from connection reuse.** Connection pooling/keep-alive is the normal default in any HTTP client library; a pooled client that reuses a request-context object or fails to reset the auth header per call can attach request A's vendor token to request B's outbound call — the same cross-tenant leak as a cache-key collision, with no cache required. This is a required implementation discipline, not an assumption left implicit.
22 - **Scoped narrowly if the vendor supports it** — request only the scope needed for `/identity`, not a broad token, so a leaked token's blast radius is bounded by the vendor's own authorization model as well as ours.
23 - **Not returned to our own API callers.** The vendor `access_token` is an internal implementation detail of this connector; question 2's API callers never see it. If a caller needs to know "identity lookup succeeded," return our own success/failure signal, not the vendor's token or vendor's raw response.
24
25 ### 2. How `/identity` calls are authenticated to the vendor
26
27 - Standard bearer-token usage: the vendor `access_token` from `/auth` presented as an `Authorization: Bearer` header to the vendor's identity endpoint (or whatever the vendor's own contract specifies — this connector adapts to the vendor, not the reverse).
28 - **Token refresh/re-auth on expiry** handled by the connector transparently — a caller of our `/identity` endpoint should not need to know or manage vendor token lifecycle.
29 - **Vendor calls over TLS only**, certificate validation never disabled, including in local/dev environments (repeating the threat model's point because it's the most common place a "temporary" dev bypass ships to production unnoticed).
30 - **Our own credentials to call the vendor's `/auth`** (this connector's client id/secret with ABC/XYZ, if the vendor requires connector-level registration in addition to the end-user's own username/password) are runtime-injected secrets, never in source or config committed to the repo — requirement set here, mechanism owned by `../04-infra-devops/CLAUDE.md`.
31
32 ### 3. What this service should and shouldn't log or persist
33
34 | Data | Log it? | Persist it? |
35 |---|---|---|
36 | End-user's vendor username | No (identifier, but low sensitivity relative to password — still avoid in plaintext logs; use it only for correlation via a hash if audit trails need it) | Only if a legitimate business reason exists; not by default |
37 | End-user's vendor password (`/auth` request body) | **Never, under any circumstance, including error paths and stack traces** | **Never** |
38 | Vendor `access_token` | **Never** | Only as short-TTL encrypted cache per above; never in plaintext, never in application logs |
39 | PII returned by `/identity` (name, phone, address fields) | Never the field values; log only the *fact* of a lookup (caller identity, timestamp, success/failure, which record was resolved by opaque id) | Retention/caching policy for this PII is `../05-data-ops/CLAUDE.md`'s call — this track's requirement is that whatever is persisted follows the same encryption-at-rest and access-control bar as `user_profile` data, no weaker because it arrived via a connector rather than direct input |
40 | Vendor error responses | Log status/error-code only; do not log raw vendor response bodies by default, since a vendor error payload can itself echo back submitted PII or partial credentials | N/A |
41
42 ### 4. Failure handling as a security control, not just reliability
43
44 - **Vendor auth failures must not leak which part of the credential was wrong** (username vs. password) in whatever this connector surfaces upward — same user-enumeration concern as the threat model's credential-stuffing row, now facing outward at whoever calls our `/auth` passthrough.
45 - **Retry/backoff on vendor calls** should be rate-limited on our side independent of the vendor's own limits, so a compromised or careless caller can't use our connector to hammer the vendor (which is both an availability risk to us and a way to get our connector's vendor credentials rate-limited or banned).
46 - **Circuit-breaking**: if the vendor is failing or behaving anomalously (e.g., returning tokens for identities that don't match the request), fail closed — do not fall back to a cached or stale identity result silently, since that risks serving stale PII as if freshly verified.
47
48 ## What this design does not resolve
49
50 - Data retention duration for cached `/identity` results — `../05-data-ops/CLAUDE.md`.
51 - Which secrets manager holds this connector's own vendor credentials — `../04-infra-devops/CLAUDE.md`.
52 - Vendor selection/evaluation (which real-world providers "ABC"/"XYZ" might represent) — that's product framing in `../01-product-industry-research-design/CLAUDE.md`, not a security question.
53
54 ## Sources
55
56 - OWASP API Security Top 10 2023, API7 (Server Side Request Forgery) and API8 (Security Misconfiguration) — both relevant to a service that makes outbound calls to a third party on a user's behalf: https://owasp.org/API-Security/editions/2023/en/0x11-t10/
57 - OWASP Logging Cheat Sheet (what not to log — credentials, tokens, PII): https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
58
59 ## Attack-tree table — connector token lifecycle (S3 red/blue, 2026-09-12)
60
61 Full positions, rebuttals, and ruling: `decisions/connector-token-lifecycle-redblue.md`. Table shape per `planning-approach.md` Appendix A.
62
63 | # | Leaf (attack path) | Precondition | Likelihood | Impact | Blue response: Control *or* Accepted risk | Owner | Residual | Source |
64 |---|---|---|---|---|---|---|---|---|
65 | 1 | Cache-key confusion → cross-tenant token/PII leak via request-derived cache key | Persistent cache (eliminated by default posture below) | M | H | **Eliminated by design:** no cache by default (Felix Adebayo, alternatives seat) — no persistent cache means no key to collide on. **Control, exception-path only:** if caching is ever re-opened, key must derive from immutable vendor-issued ids, never request-supplied `name`/`phone`. **Control (this run, Tomasz's rebuttal):** the connection-reuse variant of this leaf survives even with no cache — a pooled HTTP client that doesn't reset `Authorization` per call can attach one request's token to another. Closed with a required control: fresh, isolated auth header constructed per outbound request, never inherited from connection reuse. | 03 (client construction; cache-key derivation if exception ever opened) | **L** — storage variant eliminated; connection-reuse variant closed by an explicit, checkable implementation requirement | OWASP API8:2023; judgment (connection-reuse variant) |
66 | 2 | Fail-open on KMS unavailability during a caching window | Persistent cache with KMS-backed encryption at rest (eliminated by default posture) | M | H | **Eliminated by design:** no cache by default removes the write-time KMS dependency from the hot path. **Control, exception-path only:** if caching is ever re-opened, KMS-unreachable fails closed to "no cache, fetch live," never to an unencrypted write. Tomasz conceded this leaf outright — no hole found. | 03/04 (KMS mechanism is 04's) | **L** | NIST SP 800-57 |
67 | 3 | Vendor token replay — no proof-of-possession/session binding once decrypted for use | Token exfiltrated by any other means within its exposure window | M | H | **Accepted risk.** Bearer tokens under RFC 6750 are possession-equals-authorization by design; PoP/binding (DPoP, RFC 9449; mTLS-bound tokens, RFC 8705) needs vendor-side support this project cannot assume for a generic ABC/XYZ vendor the assignment never specifies. Acceptable *for this system* because fetch-per-call bounds the exposure window to one request's execution scope — no standing cached credential to steal. **Tightened per Tomasz's rebuttal:** the boundary is stated explicitly — fetch → use in the immediately following `/identity` call → zeroize; a retried `/identity` call re-fetches via `/auth` and never reuses a token from a prior attempt, so the accepted window cannot silently stretch to the length of the retry/backoff policy. | — (re-review trigger: a PoP-capable vendor surfacing in 01's research) | **M** — accepted, not eliminated; a real vendor-side constraint, not a design gap, but still "possession = authority" for the fetch's lifetime | RFC 6750 §5; RFC 9700 §2.5 |
68 | 4 | Process-memory/crash-dump exposure of the plaintext token before encryption | Attacker has read access to a crash artifact or swap on the host | L | M | **Control:** token field excluded from any generic request/response dump used in error handling (panic traces, debug dumps — same discipline as application logs); token variable scoped to the narrowest function possible, never passed up the call stack beyond the vendor-call function. **Residual accepted:** Go gives no guaranteed memory zeroing without unsafe/cgo, so a crash dump taken mid-call could still capture it; acceptable given the window narrowed from a cache TTL to one function call (Felix's alternatives turn). Core-dump suppression at the runtime level is a 04 mechanism, requirement recorded, not resolved here. | 03 (code discipline) / 04 (runtime config) | **L** — narrowed from a standing cache exposure to a single function-call window | OWASP Logging Cheat Sheet |
69 | 5 | Connector used as a credential-stuffing oracle against the vendor (rate-limit granularity unstated) | No per-identity/per-IP rate-limit granularity stated in the original design | M | M | **Control:** rate limit keyed per calling-API-client *and* per source IP, with a stricter threshold on vendor-auth *failure ratio* specifically (credential stuffing shows as a high failure ratio before high volume), paired with exponential backoff per identity attempted, not just per client — closes the granularity gap without adding a new requirement beyond what §4 already gestured at. | 03 (rate-limit logic) / 04 (shared rate-limit store or edge mechanism, if enforced at a gateway) | **L** | OWASP API4:2023; NIST SP 800-63B §5.2.2 |
70
71 Rules of use followed: Tomasz ranked and handed over his top five before Helena answered (per `CASTING.md` §5 ⟨02⟩); Felix's alternatives turn sat between red and blue and eliminated leaves 1's storage variant and 2's substrate by design change rather than control; Helena answered every leaf with a control or a justified accepted risk; Tomasz's rebuttal turn on the top three surfaced one unaddressed variant (leaf 1's connection-reuse case) and one boundary needing explicit statement (leaf 3), both resolved without reopening the ruling; Marcus assigned Residual with the justification in each row. No Residual **H** — none of the five leaves cleared that bar after blue's response and the rebuttal-turn tightening, so no follow-up Story is required; leaf 3's **M** is the one residual worth Warren's attention as a genuine accepted risk, not a gap.
72
73 ---
74 Model: sonnet (Tomasz Wrede, red; Felix Adebayo, alternatives; Helena Marsh, blue) / sonnet (Marcus Ilori, lead, ruling and residual scoring). Turns consumed: 5 (red top-five, alternatives, blue, rebuttal, lead residual scoring) — matches the 4-turns-plus-lead budget in `PLAN.md` §3.
75 {
"command": "python3 ~/.claude/skills/profile-gen/scripts/show_profile.py --profile ./profiles/ingrid-solano/ingrid-solano.md --root ."
}{"name": "Ingrid Solano", "mode": "hud", "hud": true, "badge": false, "background": false, "inline": false}{
"command": "gh pr diff 26 --repo wtvamp/loginid-takehome 2>&1 | head -400"
}diff --git a/internal/api/auth.go b/internal/api/auth.go
new file mode 100644
index 0000000..349ef97
--- /dev/null
+++ b/internal/api/auth.go
@@ -0,0 +1,103 @@
+// Package api implements the Q2 REST handlers (search/retrieve
+// user_profile) per 02-ai-security-architecture/handoff-03-auth.md v3 and
+// 03-engineering-delivery/refinement/LT-39.md. This story's own non-goals
+// (LT-39, S6) exclude the JWT verification middleware itself — that's S7
+// (LT-40), a separate pull — so every handler here reads its auth context
+// from the request's context.Context, populated by whatever put it there.
+// Until S7 lands, that's this package's own tests using fakes, per
+// decisions/test-double-strategy.md; once S7 lands, it's S7's middleware.
+// These handlers are not yet wired into cmd/api-service's live router —
+// doing so before S7 exists would expose unauthenticated PII, which is
+// exactly the outcome the whole auth design exists to prevent.
+package api
+
+import "context"
+
+// Scope is the token's authorized action, from handoff-03-auth.md v3's
+// closed vocabulary (§ "Scope vocabulary"). A typed string rather than a
+// bare one so a handler comparing scopes can't typo past the compiler on
+// a scope it never validates against a literal.
+type Scope string
+
+const (
+ // ScopeReadOwn retrieves a single record by id the caller already
+ // holds a legitimate reference to; authorize() checks the target
+ // against the caller_referral mapping.
+ ScopeReadOwn Scope = "profile:read:own"
+ // ScopeReadAny retrieves a single record by id with no referral
+ // constraint, but requires a ReasonCode on every request and is
+ // still subject to the cumulative touch cap.
+ ScopeReadAny Scope = "profile:read:any"
+ // ScopeSearch queries by name/phone fragment across the table. The
+ // broadest scope; there is no broader one.
+ ScopeSearch Scope = "profile:search"
+)
+
+// AuthContext is what a validated request carries once past S7's
+// middleware (or, until S7 exists, whatever a test injects directly via
+// WithAuthContext). Sub is the calling client identity (the token's `sub`
+// claim); Scope is the single scope this request is being made under —
+// handoff-03-auth.md v3 doesn't describe multi-scope tokens being
+// disambiguated per-request, so one request carries exactly one Scope.
+type AuthContext struct {
+ Sub string
+ Scope Scope
+}
+
+type authContextKey struct{}
+
+// WithAuthContext attaches ac to ctx. The seam S7's middleware (or a
+// test) uses to populate the auth context every handler in this package
+// reads.
+func WithAuthContext(ctx context.Context, ac AuthContext) context.Context {
+ return context.WithValue(ctx, authContextKey{}, ac)
+}
+
+// AuthContextFromContext retrieves the AuthContext WithAuthContext
+// attached, ok=false if none was ever set (e.g. this package's handlers
+// were reached without going through auth middleware — a caller bug, not
+// a request the handler should try to interpret).
+func AuthContextFromContext(ctx context.Context) (AuthContext, bool) {
+ ac, ok := ctx.Value(authContextKey{}).(AuthContext)
+ return ac, ok
+}
+
+// ReasonCode is the closed enum profile:read:any requires on every
+// request (handoff-03-auth.md v3). Adding a value is a documented change
+// to this list, not a runtime config toggle.
+type ReasonCode string
+
+const (
+ ReasonSupportTicket ReasonCode = "support_ticket"
+ ReasonFraudReview ReasonCode = "fraud_review"
+ ReasonKYCReverification ReasonCode = "kyc_reverification"
+ ReasonLegalHold ReasonCode = "legal_hold"
+)
+
+// Valid reports whether r is one of the four closed enum values.
+func (r ReasonCode) Valid() bool {
+ switch r {
+ case ReasonSupportTicket, ReasonFraudReview, ReasonKYCReverification, ReasonLegalHold:
+ return true
+ default:
+ return false
+ }
+}
+
+// Authorizer is the object-level policy hook (handoff-03-auth.md v3):
+// "a function authorize(sub, scope, target) -> allow|deny, called after
+// scope validation, before the handler executes the query/retrieve."
+// caller_referral and the caller-role table live in the token-issuing
+// authorization server's own datastore (v3's F8 resolution) — not
+// 05-data-ops's schema, and not built by this story. This interface is
+// the seam: real implementation is a follow-up story against that
+// datastore; this package's tests use a hand-written fake per
+// decisions/test-double-strategy.md.
+type Authorizer interface {
+ // Authorize reports whether sub may act under scope against target
+ // (a record id for ScopeReadOwn/ScopeReadAny; ignored for
+ // ScopeSearch, which has no single target). A false, nil return is
+ // an ordinary policy denial, not an error — errors are reserved for
+ // the datastore itself being unreachable.
+ Authorize(ctx context.Context, sub string, scope Scope, target string) (bool, error)
+}
diff --git a/internal/api/cursor.go b/internal/api/cursor.go
new file mode 100644
index 0000000..28abe91
--- /dev/null
+++ b/internal/api/cursor.go
@@ -0,0 +1,49 @@
+package api
+
+import (
+ "encoding/base64"
+ "fmt"
+ "strconv"
+ "strings"
+)
+
+// cursorPrefix distinguishes this package's cursors from arbitrary
+// base64 a caller might send, so a malformed cursor fails with a clear
+// message rather than a confusing offset.
+const cursorPrefix = "v1:"
+
+// encodeCursor produces the opaque, API-facing cursor for the next page
+// starting at offset. handoff-03-auth.md v3's F-pag correction: the
+// cursor may encode the DAO's own Offset directly underneath — the
+// opaque-cursor requirement is that callers cannot construct or walk one
+// by hand, not that the handler is forbidden from using Offset
+// internally. Encoding is base64 of a versioned, prefixed string rather
+// than the bare decimal offset so a caller can't trivially read or
+// increment it by eye.
+func encodeCursor(offset int) string {
+ return base64.RawURLEncoding.EncodeToString([]byte(cursorPrefix + strconv.Itoa(offset)))
+}
+
+// decodeCursor recovers the offset encodeCursor produced. An empty
+// cursor (the first page) decodes to offset 0, not an error.
+func decodeCursor(cursor string) (offset int, err error) {
+ if cursor == "" {
+ return 0, nil
+ }
+ raw, err := base64.RawURLEncoding.DecodeString(cursor)
+ if err != nil {
+ return 0, fmt.Errorf("api: malformed cursor: %w", err)
+ }
+ s := string(raw)
+ if !strings.HasPrefix(s, cursorPrefix) {
+ return 0, fmt.Errorf("api: malformed cursor: missing version prefix")
+ }
+ n, err := strconv.Atoi(strings.TrimPrefix(s, cursorPrefix))
+ if err != nil {
+ return 0, fmt.Errorf("api: malformed cursor: %w", err)
+ }
+ if n < 0 {
+ return 0, fmt.Errorf("api: malformed cursor: negative offset")
+ }
+ return n, nil
+}
diff --git a/internal/api/cursor_test.go b/internal/api/cursor_test.go
new file mode 100644
index 0000000..ecac38c
--- /dev/null
+++ b/internal/api/cursor_test.go
@@ -0,0 +1,60 @@
+package api
+
+import (
+ "encoding/base64"
+ "testing"
+)
+
+func TestCursor_RoundTrip(t *testing.T) {
+ for _, offset := range []int{0, 1, 20, 12345} {
+ c := encodeCursor(offset)
+ got, err := decodeCursor(c)
+ if err != nil {
+ t.Fatalf("decodeCursor(%q): %v", c, err)
+ }
+ if got != offset {
+ t.Errorf("round trip of offset %d = %d", offset, got)
+ }
+ }
+}
+
+func TestCursor_EmptyIsOffsetZero(t *testing.T) {
+ got, err := decodeCursor("")
+ if err != nil {
+ t.Fatalf("decodeCursor(\"\"): %v", err)
+ }
+ if got != 0 {
+ t.Errorf("empty cursor decoded to %d, want 0", got)
+ }
+}
+
+func TestCursor_OpaqueAtAPIBoundary(t *testing.T) {
+ // A caller cannot construct or walk a cursor by hand — the encoded
+ // form must not be a bare decimal offset a caller could increment by
+ // eye (handoff-03-auth.md v3's opaque-cursor requirement).
+ c := encodeCursor(20)
+ if c == "20" {
+ t.Error("cursor must not be the bare decimal offset")
+ }
+}
+
+func TestCursor_MalformedRejected(t *testing.T) {
+ cases := []string{"not-base64!!!", "AAAA", "0"}
+ for _, c := range cases {
+ if _, err := decodeCursor(c); err == nil {
+ t.Errorf("decodeCursor(%q) = nil error, want a malformed-cursor error", c)
+ }
+ }
+}
+
+// TestCursor_NegativeOffsetRejected constructs a tampered cursor directly
+// (base64 of "v1:-5") — encodeCursor never produces a negative offset
+// itself, so this proves decodeCursor's own guard against untrusted
+// caller input, not encodeCursor's behavior.
+func TestCursor_NegativeOffsetRejected(t *testing.T) {
+ tampered := base64.RawURLEncoding.EncodeToString([]byte("v1:-5"))
+ neg, err := decodeCursor(tampered)
+ if err == nil {
+ t.Errorf("decodeCursor of a negative-offset payload = %d, nil error, want rejected", neg)
+ }
+}
diff --git a/internal/api/errors.go b/internal/api/errors.go
new file mode 100644
index 0000000..1ea258f
--- /dev/null
+++ b/internal/api/errors.go
@@ -0,0 +1,98 @@
+// errors.go implements this package's boundary-translation function, per
+// decisions/error-semantics.md: internal/api never serializes err.Error()
+// on an unmapped error. Only the DAO's seven sentinels' safe messages
+// (plus the authn/authz/context rules below) ever reach a client; anything
+// else defaults to a generic 500 with no wrapped detail.
+package api
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "net/http"
+
+ "loginid-takehome/internal/dao"
+)
+
+// errorResponse is the only JSON shape an error path ever writes. code is
+// a short machine-readable string (never a driver error, never
+// err.Error()); message is a safe, human-readable sentence.
+type errorResponse struct {
+ Code string `json:"code"`
+ Message string `json:"message"`
+}
+
+func writeError(w http.ResponseWriter, status int, code, message string) {
+ w.Header().Set("Content-Type", "application/json")
+ w.WriteHeader(status)
+ _ = json.NewEncoder(w).Encode(errorResponse{Code: code, Message: message})
+}
+
+// writeUnauthorized and writeForbidden are named separately from the
+// generic mapper below because F7 (decisions/error-semantics.md) requires
+// an identical client-visible body for a scope failure and an
+// authorize() policy denial — the two are distinguished only in the
+// audit log, never in what the client sees. Both call the one function
+// below so that identity is enforced by construction, not by two call
+// sites happening to agree.
+func writeUnauthorized(w http.ResponseWriter) {
+ writeError(w, http.StatusUnauthorized, "unauthorized", "missing or invalid token")
+}
+
+func writeForbidden(w http.ResponseWriter) {
+ writeError(w, http.StatusForbidden, "forbidden", "not authorized for this request")
+}
+
+func writeRateLimited(w http.ResponseWriter) {
+ writeError(w, http.StatusTooManyRequests, "rate_limited", "rate limit exceeded")
+}
+
+func writeTouchCapExceeded(w http.ResponseWriter) {
+ writeError(w, http.StatusTooManyRequests, "cumulative_cap_exceeded", "cumulative record-access cap exceeded for the current window")
+}
+
+func writeBadRequest(w http.ResponseWriter, message string) {
+ writeError(w, http.StatusBadRequest, "bad_request", message)
+}
+
+// writeDAOError translates an error returned by the DAO layer into an
+// HTTP response, per decisions/error-semantics.md's boundary rule and its
+// F7/F50 additions:
+// - context.DeadlineExceeded -> 504, never wrapped in a generic 500.
+// - context.Canceled -> handled by the caller before this is reached
+// (a cancelled client request has no response to write); if it
+// reaches here anyway it's treated the same as an unmapped error but
+// the caller should not count it as an alerting failure.
+// - one of the seven dao sentinels -> its mapped status/safe message.
+// - anything else -> 500, no wrapped detail, ever.
+func writeDAOError(w http.ResponseWriter, err error) {
+ switch {
+ case errors.Is(err, context.DeadlineExceeded):
+ writeError(w, http.StatusGatewayTimeout, "deadline_exceeded", "request timed out")
+ case errors.Is(err, dao.ErrNotFound):
+ writeError(w, http.StatusNotFound, "not_found", "no such record")
+ case errors.Is(err, dao.ErrInvalidArgument):
+ writeBadRequest(w, "invalid request")
+ case errors.Is(err, dao.ErrInvalidQuery):
+ writeBadRequest(w, "invalid search query")
+ case errors.Is(err, dao.ErrDuplicateUsername),
+ errors.Is(err, dao.ErrInvalidMethod),
+ errors.Is(err, dao.ErrInvalidCredential),
+ errors.Is(err, dao.ErrAlreadyExists):
+ // None of these are reachable from this package's read-only
+ // search/retrieve handlers (they're write-path sentinels), but
+ // mapped defensively rather than falling through to a raw 500
+ // if a future handler in this package ever does reach them.
+ writeBadRequest(w, "request could not be completed")
+ default:
+ // Never err.Error() here — an unmapped error (a raw driver
+ // error that somehow escaped the DAO's translation layer, a
+ // context.Canceled that reached this far, anything else) gets a
+ // generic message with no wrapped detail (decisions/
+ // error-semantics.md's F50: no DETAIL text, no driver message,
+ // ever, ***even in the log line a caller can't see*** — that
+ // sanitization is the DAO translation layer's job upstream of
+ // here, not repeated in this file).
+ writeError(w, http.StatusInternalServerError, "internal_error", "internal error")
+ }
+}
diff --git a/internal/api/fakes_test.go b/internal/api/fakes_test.go
new file mode 100644
index 0000000..3270e18
--- /dev/null
+++ b/internal/api/fakes_test.go
@@ -0,0 +1,112 @@
+package api
+
+import (
+ "context"
+ "time"
+
+ "loginid-takehome/internal/dao"
+ "loginid-takehome/internal/model"
+)
+
+// Hand-written fakes, per decisions/test-double-strategy.md: any change
+// to Repository or its three sub-interfaces must update the corresponding
+// fake in the same PR — a review-checklist item, not codegen.
+
+// fakeRepository implements dao.Repository. Only Profiles() is exercised
+// by this package's handlers; the rest panic if ever called, so a test
+// that reaches them fails loudly instead of silently zero-valuing.
+type fakeRepository struct {
+ profiles fakeProfileRepository
+}
+
+func (f *fakeRepository) Profiles() dao.ProfileRepository { return &f.profiles }
+func (f *fakeRepository) Credentials() dao.CredentialRepository { panic("not used by internal/api") }
+func (f *fakeRepository) Methods() dao.AuthMethodRepository { panic("not used by internal/api") }
+func (f *fakeRepository) CreateProfileWithCredential(ctx context.Context, p *model.UserProfile, c *model.UserCredential) (*model.UserProfile, *model.UserCredential, error) {
+ panic("not used by internal/api")
+}
+func (f *fakeRepository) DeleteExpired(ctx context.Context, class dao.RetentionClass, olderThan time.Time, maxRows int) (dao.SweepResult, error) {
+ panic("not used by internal/api")
+}
+func (f *fakeRepository) DeleteProfile(ctx context.Context, id string, externalRef *string) error {
+ panic("not used by internal/api")
+}
+func (f *fakeRepository) Close() error { return nil }
+
+// fakeProfileRepository implements dao.ProfileRepository with
+// configurable funcs per test, per the ruled test-double strategy.
+type fakeProfileRepository struct {
+ getFn func(ctx context.Context, id string) (*model.UserProfile, error)
+ searchFn func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error)
+}
+
+func (f *fakeProfileRepository) Get(ctx context.Context, id string) (*model.UserProfile, error) {
+ return f.getFn(ctx, id)
+}
+func (f *fakeProfileRepository) Search(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ return f.searchFn(ctx, q)
+}
+func (f *fakeProfileRepository) Create(ctx context.Context, p *model.UserProfile) (*model.UserProfile, error) {
+ panic("not used by internal/api")
+}
+func (f *fakeProfileRepository) Update(ctx context.Context, p *model.UserProfile) (*model.UserProfile, error) {
+ panic("not used by internal/api")
+}
+func (f *fakeProfileRepository) Upsert(ctx context.Context, p *model.UserProfile) (*model.UserProfile, error) {
+ panic("not used by internal/api")
+}
+func (f *fakeProfileRepository) Delete(ctx context.Context, id string) error {
+ panic("not used by internal/api")
+}
<system-reminder>GitHub API rate limit exceeded (5,000/hr shared across all tools and agents). Run `gh api rate_limit --jq .resources` and sleep until reset before further gh calls. If polling in a loop, use ScheduleWakeup instead of retrying.</system-reminder>{
"command": "gh api rate_limit --jq .resources 2>&1"
}{"actions_runner_registration":{"limit":10000,"remaining":10000,"reset":1789278770,"used":0},"audit_log":{"limit":1750,"remaining":1750,"reset":1789278770,"used":0},"audit_log_streaming":{"limit":15,"remaining":15,"reset":1789278770,"used":0},"code_scanning_autofix":{"limit":10,"remaining":10,"reset":1789275230,"used":0},"code_search":{"limit":10,"remaining":10,"reset":1789275230,"used":0},"copilot_usage_records":{"limit":1750,"remaining":1750,"reset":1789278770,"used":0},"core":{"limit":5000,"remaining":5000,"reset":1789278770,"used":0},"dependency_sbom":{"limit":100,"remaining":100,"reset":1789275230,"used":0},"dependency_snapshots":{"limit":100,"remaining":100,"reset":1789275230,"used":0},"enterprise_token_inventory":{"limit":1750,"remaining":1750,"reset":1789278770,"used":0},"graphql":{"limit":5000,"remaining":5000,"reset":1789278770,"used":0},"integration_manifest":{"limit":5000,"remaining":5000,"reset":1789278770,"used":0},"scim":{"limit":15000,"remaining":15000,"reset":1789278770,"used":0},"search":{"limit":30,"remaining":30,"reset":1789275230,"used":0},"source_import":{"limit":100,"remaining":100,"reset":1789275230,"used":0}}{
"command": "gh pr diff 26 --repo wtvamp/loginid-takehome > /tmp/pr26.diff 2>&1; wc -l /tmp/pr26.diff"
}1481 /tmp/pr26.diff
{
"command": "mkdir -p /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad; cp /tmp/pr26.diff /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad/pr26.diff; sed -n '400,750p' /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad/pr26.diff"
}+}
+
+// fakeAuthorizer implements Authorizer with a configurable decision.
+type fakeAuthorizer struct {
+ allow bool
+ err error
+}
+
+func (f *fakeAuthorizer) Authorize(ctx context.Context, sub string, scope Scope, target string) (bool, error) {
+ return f.allow, f.err
+}
+
+// fakeRateLimiter implements RateLimiter with a configurable decision —
+// tests exercising the rate-limit path don't need real wall-clock timing.
+type fakeRateLimiter struct {
+ allow bool
+ err error
+}
+
+func (f *fakeRateLimiter) Allow(ctx context.Context, sub string, scope Scope, limit int) (bool, error) {
+ return f.allow, f.err
+}
+
+// fakeTouchCounter implements TouchCounter with configurable behavior.
+type fakeTouchCounter struct {
+ allowed bool
+ allowedErr error
+ recordFn func(ctx context.Context, sub string, recordIDs []string) (int, error)
+ recordedIDs []string
+}
+
+func (f *fakeTouchCounter) Allowed(ctx context.Context, sub string) (bool, error) {
+ return f.allowed, f.allowedErr
+}
+func (f *fakeTouchCounter) RecordTouches(ctx context.Context, sub string, recordIDs []string) (int, error) {
+ f.recordedIDs = append(f.recordedIDs, recordIDs...)
+ if f.recordFn != nil {
+ return f.recordFn(ctx, sub, recordIDs)
+ }
+ return len(f.recordedIDs), nil
+}
+
+// allowAllDeps builds a Deps whose collaborators all say yes — the
+// baseline for a test that then overrides exactly one collaborator to
+// exercise its own denial path.
+func allowAllDeps() Deps {
+ return Deps{
+ Repo: &fakeRepository{},
+ Authz: &fakeAuthorizer{allow: true},
+ RateLimiter: &fakeRateLimiter{allow: true},
+ TouchCounter: &fakeTouchCounter{allowed: true},
+ }
+}
diff --git a/internal/api/handlers.go b/internal/api/handlers.go
new file mode 100644
index 0000000..b094e4f
--- /dev/null
+++ b/internal/api/handlers.go
@@ -0,0 +1,260 @@
+package api
+
+import (
+ "encoding/json"
+ "net/http"
+ "strings"
+
+ "loginid-takehome/internal/dao"
+)
+
+// Deps bundles a handler's collaborators — the DAO, the object-level
+// policy hook, and the two independent limiters — so NewSearchHandler/
+// NewGetProfileHandler take one argument instead of four, and a test
+// fake for any one of them doesn't require constructing the other three
+// by hand each time.
+type Deps struct {
+ Repo dao.Repository
+ Authz Authorizer
+ RateLimiter RateLimiter
+ TouchCounter TouchCounter
+}
+
+const (
+ defaultPageSize = 20
+ maxPageSize = 50
+ readRateLimit = 60 // req/min, read:own and read:any
+ searchRateLimit = 10 // req/min, profile:search
+)
+
+type searchRequest struct {
+ Name *string `json:"name,omitempty"`
+ Phone *string `json:"phone,omitempty"`
+ Region *string `json:"region,omitempty"`
+ Country *string `json:"country,omitempty"`
+ Cursor string `json:"cursor,omitempty"`
+ PageSize int `json:"page_size,omitempty"`
+}
+
+type searchResponse struct {
+ Results []SearchResultProfile `json:"results"`
+ Total int `json:"total"`
+ NextCursor string `json:"next_cursor,omitempty"`
+}
+
+// NewSearchHandler implements profile:search (handoff-03-auth.md v3).
+// Order of checks, deliberately: auth context present -> scope is
+// ScopeSearch -> per-minute rate limit -> cumulative touch cap -> request
+// shape (empty body, bare-wildcard-shaped unconstrained query) ->
+// authorize() -> the DAO call -> record touches -> masked response. Scope
+// mismatch and an authorize() denial return an identical body (F7); only
+// the audit log (not built by this story — see refinement/LT-39.md's
+// non-goals) would distinguish them.
+func NewSearchHandler(d Deps) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ ctx := r.Context()
+ ac, ok := AuthContextFromContext(ctx)
+ if !ok {
+ writeUnauthorized(w)
+ return
+ }
+ if ac.Scope != ScopeSearch {
+ writeForbidden(w)
+ return
+ }
+
+ allow, err := d.RateLimiter.Allow(ctx, ac.Sub, ScopeSearch, searchRateLimit)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !allow {
+ writeRateLimited(w)
+ return
+ }
+
+ capOK, err := d.TouchCounter.Allowed(ctx, ac.Sub)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !capOK {
+ writeTouchCapExceeded(w)
+ return
+ }
+
+ var req searchRequest
+ if r.Body != nil {
+ if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+ writeBadRequest(w, "malformed request body")
+ return
+ }
+ }
+
+ // A search request with no filter field at all is rejected here
+ // with a 400 before it ever reaches the DAO — the DAO's own
+ // all-nil ProfileQuery is legal and returns every profile, which
+ // this API must not expose directly (handoff-03-auth.md v3, F10).
+ if req.Name == nil && req.Phone == nil && req.Region == nil && req.Country == nil {
+ writeBadRequest(w, "at least one search filter is required")
+ return
+ }
+
+ offset, err := decodeCursor(req.Cursor)
+ if err != nil {
+ writeBadRequest(w, "malformed cursor")
+ return
+ }
+
+ pageSize := req.PageSize
+ if pageSize <= 0 {
+ pageSize = defaultPageSize
+ }
+ if pageSize > maxPageSize {
+ // A bare/unconstrained request for more than this API's
+ // result cap is rejected outright, not truncated
+ // (handoff-03-auth.md v3) — checked here, before the query
+ // is built, mirroring the same rule for authorize() below.
+ writeBadRequest(w, "page_size exceeds the maximum of 50")
+ return
+ }
+
+ // The object-level policy hook, per v3: called after scope
+ // validation (above) and request-shape validation (above), before
+ // the DAO call. ScopeSearch has no single target id.
+ allowed, err := d.Authz.Authorize(ctx, ac.Sub, ac.Scope, "")
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !allowed {
+ writeForbidden(w)
+ return
+ }
+
+ q := dao.ProfileQuery{
+ Name: req.Name,
+ Phone: req.Phone,
+ Region: req.Region,
+ Country: req.Country,
+ Limit: pageSize,
+ Offset: offset,
+ }
+ results, total, err := d.Repo.Profiles().Search(ctx, q)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+
+ ids := make([]string, len(results))
+ masked := make([]SearchResultProfile, len(results))
+ for i, p := range results {
+ ids[i] = p.ID
+ masked[i] = toSearchResult(p)
+ }
+ if _, err := d.TouchCounter.RecordTouches(ctx, ac.Sub, ids); err != nil {
+ writeDAOError(w, err)
+ return
+ }
+
+ resp := searchResponse{Results: masked, Total: total}
+ if offset+len(results) < total {
+ resp.NextCursor = encodeCursor(offset + len(results))
+ }
+ writeJSON(w, http.StatusOK, resp)
+ }
+}
+
+// NewGetProfileHandler implements profile:read:own and profile:read:any
+// against the same handler — the two scopes differ only in what
+// Authorize() checks (referral membership vs. a logged reason_code), not
+// in the handler's own shape. id is read from the request path via
+// r.PathValue("id") (Go 1.22+ ServeMux pattern, e.g.
+// "GET /profiles/{id}"); this handler is not yet registered on any real
+// route (refinement/LT-39.md's Acceptance outcome — deferred until S7's
+// auth middleware exists to protect it), so tests call it directly with
+// http.Request.SetPathValue.
+func NewGetProfileHandler(d Deps) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ ctx := r.Context()
+ ac, ok := AuthContextFromContext(ctx)
+ if !ok {
+ writeUnauthorized(w)
+ return
+ }
+ if ac.Scope != ScopeReadOwn && ac.Scope != ScopeReadAny {
+ writeForbidden(w)
+ return
+ }
+
+ id := r.PathValue("id")
+ if strings.TrimSpace(id) == "" {
+ writeBadRequest(w, "id is required")
+ return
+ }
+
+ var reasonCode ReasonCode
+ if ac.Scope == ScopeReadAny {
+ reasonCode = ReasonCode(r.URL.Query().Get("reason_code"))
+ if !reasonCode.Valid() {
+ writeBadRequest(w, "reason_code is required for profile:read:any and must be one of the closed enum values")
+ return
+ }
+ }
+
+ allow, err := d.RateLimiter.Allow(ctx, ac.Sub, ac.Scope, readRateLimit)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !allow {
+ writeRateLimited(w)
+ return
+ }
+
+ // Applies to profile:read:any unconditionally, whether or not a
+ // reason_code is present — the exact regression handoff-03-auth.md
+ // v3 fixed (an earlier wording made this counter a no-op for
+ // read:any, since that scope requires a reason_code on every call
+ // by design; a valid, logged reason_code satisfies Authorize()
+ // below but does not exempt the caller from this cap).
+ capOK, err := d.TouchCounter.Allowed(ctx, ac.Sub)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !capOK {
+ writeTouchCapExceeded(w)
+ return
+ }
+
+ allowed, err := d.Authz.Authorize(ctx, ac.Sub, ac.Scope, id)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+ if !allowed {
+ writeForbidden(w)
+ return
+ }
+
+ p, err := d.Repo.Profiles().Get(ctx, id)
+ if err != nil {
+ writeDAOError(w, err)
+ return
+ }
+
+ if _, err := d.TouchCounter.RecordTouches(ctx, ac.Sub, []string{p.ID}); err != nil {
+ writeDAOError(w, err)
+ return
+ }
+
+ writeJSON(w, http.StatusOK, toProfileResponse(p))
+ }
+}
+
+func writeJSON(w http.ResponseWriter, status int, body any) {
+ w.Header().Set("Content-Type", "application/json")
+ w.WriteHeader(status)
+ _ = json.NewEncoder(w).Encode(body)
+}
diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go
new file mode 100644
index 0000000..112e50e
--- /dev/null
+++ b/internal/api/handlers_test.go
@@ -0,0 +1,319 @@
+package api
+
+import (
+ "context"
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "loginid-takehome/internal/dao"
+ "loginid-takehome/internal/model"
+)
+
+func withScope(sub string, scope Scope) context.Context {
+ return WithAuthContext(context.Background(), AuthContext{Sub: sub, Scope: scope})
+}
+
+func strp(s string) *string { return &s }
+
+// ---- profile:search ----
+
+func TestSearchHandler_MissingAuthContext_401(t *testing.T) {
+ h := NewSearchHandler(allowAllDeps())
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ w := httptest.NewRecorder(){
"command": "grep -n \"^diff --git\" /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad/pr26.diff"
}1:diff --git a/internal/api/auth.go b/internal/api/auth.go 110:diff --git a/internal/api/cursor.go b/internal/api/cursor.go 165:diff --git a/internal/api/cursor_test.go b/internal/api/cursor_test.go 231:diff --git a/internal/api/errors.go b/internal/api/errors.go 335:diff --git a/internal/api/fakes_test.go b/internal/api/fakes_test.go 453:diff --git a/internal/api/handlers.go b/internal/api/handlers.go 719:diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go 1044:diff --git a/internal/api/masking.go b/internal/api/masking.go 1129:diff --git a/internal/api/masking_test.go b/internal/api/masking_test.go 1160:diff --git a/internal/api/ratelimit.go b/internal/api/ratelimit.go 1228:diff --git a/internal/api/ratelimit_test.go b/internal/api/ratelimit_test.go 1299:diff --git a/internal/api/touchcounter.go b/internal/api/touchcounter.go 1395:diff --git a/internal/api/touchcounter_test.go b/internal/api/touchcounter_test.go
{
"command": "sed -n '719,1481p' /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad/pr26.diff"
}diff --git a/internal/api/handlers_test.go b/internal/api/handlers_test.go
new file mode 100644
index 0000000..112e50e
--- /dev/null
+++ b/internal/api/handlers_test.go
@@ -0,0 +1,319 @@
+package api
+
+import (
+ "context"
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "loginid-takehome/internal/dao"
+ "loginid-takehome/internal/model"
+)
+
+func withScope(sub string, scope Scope) context.Context {
+ return WithAuthContext(context.Background(), AuthContext{Sub: sub, Scope: scope})
+}
+
+func strp(s string) *string { return &s }
+
+// ---- profile:search ----
+
+func TestSearchHandler_MissingAuthContext_401(t *testing.T) {
+ h := NewSearchHandler(allowAllDeps())
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusUnauthorized {
+ t.Errorf("status = %d, want 401", w.Code)
+ }
+}
+
+func TestSearchHandler_WrongScope_403(t *testing.T) {
+ h := NewSearchHandler(allowAllDeps())
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ req = req.WithContext(withScope("client1", ScopeReadOwn))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusForbidden {
+ t.Errorf("status = %d, want 403", w.Code)
+ }
+}
+
+// TestSearchHandler_EmptyBody_400 is LT-39's acceptance criterion: "a
+// search request with no filter field at all is rejected by this handler
+// with a 400 before it reaches the DAO."
+func TestSearchHandler_EmptyBody_400(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.searchFn = func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ t.Fatal("Search must not be called for an empty request body")
+ return nil, 0, nil
+ }
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusBadRequest {
+ t.Errorf("status = %d, want 400", w.Code)
+ }
+}
+
+// TestSearchHandler_PageSizeAboveCap_RejectedNotTruncated is LT-39's
+// "rejected outright, not truncated" criterion, checked before the query
+// is built.
+func TestSearchHandler_PageSizeAboveCap_RejectedNotTruncated(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.searchFn = func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ t.Fatal("Search must not be called when page_size exceeds the cap")
+ return nil, 0, nil
+ }
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a","page_size":51}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusBadRequest {
+ t.Errorf("status = %d, want 400", w.Code)
+ }
+}
+
+// TestSearchHandler_ResponseIsMasked asserts the search-response
+// serialization path is distinct from the read-response one: the JSON
+// body must carry the masked phone shape and must never carry
+// street_address/postal_code/country at all.
+func TestSearchHandler_ResponseIsMasked(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ phone := "+15551234567"
+ street := "123 Main St"
+ repo.profiles.searchFn = func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ return []model.UserProfile{{ID: "p1", Name: "Jane Doe", Phone: &phone, StreetAddress: &street, Region: strp("CA")}}, 1, nil
+ }
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"jane"}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusOK {
+ t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String())
+ }
+ var body map[string]any
+ if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+ t.Fatalf("decoding response: %v", err)
+ }
+ results, _ := body["results"].([]any)
+ if len(results) != 1 {
+ t.Fatalf("results = %v, want 1 entry", results)
+ }
+ result := results[0].(map[string]any)
+ if result["phone"] != "***-***-4567" {
+ t.Errorf("phone = %v, want masked", result["phone"])
+ }
+ if _, ok := result["street_address"]; ok {
+ t.Errorf("search response must never carry street_address, got %v", result)
+ }
+}
+
+func TestSearchHandler_RateLimited_429(t *testing.T) {
+ d := allowAllDeps()
+ d.RateLimiter = &fakeRateLimiter{allow: false}
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusTooManyRequests {
+ t.Errorf("status = %d, want 429", w.Code)
+ }
+}
+
+// TestSearchHandler_TouchCapExceeded_429NotServed is the PO review
+// script's item 4a: a call made after the threshold is crossed returns
+// 429 and is NOT served — not merely observed to exceed the threshold.
+func TestSearchHandler_TouchCapExceeded_429NotServed(t *testing.T) {
+ d := allowAllDeps()
+ d.TouchCounter = &fakeTouchCounter{allowed: false}
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.searchFn = func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ t.Fatal("Search must not be called once the touch cap is already exceeded")
+ return nil, 0, nil
+ }
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusTooManyRequests {
+ t.Errorf("status = %d, want 429", w.Code)
+ }
+}
+
+func TestSearchHandler_AuthorizeDenied_403(t *testing.T) {
+ d := allowAllDeps()
+ d.Authz = &fakeAuthorizer{allow: false}
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusForbidden {
+ t.Errorf("status = %d, want 403", w.Code)
+ }
+}
+
+func TestSearchHandler_DAOError_NeverLeaksRawMessage(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.searchFn = func(ctx context.Context, q dao.ProfileQuery) ([]model.UserProfile, int, error) {
+ return nil, 0, errUnmapped
+ }
+ h := NewSearchHandler(d)
+ req := httptest.NewRequest(http.MethodPost, "/profiles/search", strings.NewReader(`{"name":"a"}`))
+ req = req.WithContext(withScope("client1", ScopeSearch))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusInternalServerError {
+ t.Errorf("status = %d, want 500", w.Code)
+ }
+ if strings.Contains(w.Body.String(), "supersecret-driver-detail") {
+ t.Errorf("response leaked the raw error: %s", w.Body.String())
+ }
+}
+
+var errUnmapped = &sentinelStringer{"supersecret-driver-detail: DETAIL Key (username)=(x) already exists"}
+
+type sentinelStringer struct{ s string }
+
+func (e *sentinelStringer) Error() string { return e.s }
+
+// ---- profile:read:own / profile:read:any ----
+
+func newGetRequest(id string) *http.Request {
+ req := httptest.NewRequest(http.MethodGet, "/profiles/"+id, nil)
+ req.SetPathValue("id", id)
+ return req
+}
+
+func TestGetProfileHandler_ReadOwn_Success(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.getFn = func(ctx context.Context, id string) (*model.UserProfile, error) {
+ return &model.UserProfile{ID: id, Name: "Jane Doe"}, nil
+ }
+ h := NewGetProfileHandler(d)
+ req := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req = req.WithContext(withScope("client1", ScopeReadOwn))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusOK {
+ t.Fatalf("status = %d, want 200, body=%s", w.Code, w.Body.String())
+ }
+}
+
+// TestGetProfileHandler_ReadAny_RequiresValidReasonCode covers the closed
+// enum requirement.
+func TestGetProfileHandler_ReadAny_RequiresValidReasonCode(t *testing.T) {
+ d := allowAllDeps()
+ h := NewGetProfileHandler(d)
+ req := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req = req.WithContext(withScope("client1", ScopeReadAny))
+ w := httptest.NewRecorder()
+ h(w, req) // no reason_code query param at all
+ if w.Code != http.StatusBadRequest {
+ t.Errorf("status = %d, want 400 when reason_code is missing", w.Code)
+ }
+
+ req2 := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req2.URL.RawQuery = "reason_code=not_a_real_reason"
+ req2 = req2.WithContext(withScope("client1", ScopeReadAny))
+ w2 := httptest.NewRecorder()
+ h(w2, req2)
+ if w2.Code != http.StatusBadRequest {
+ t.Errorf("status = %d, want 400 for an unrecognized reason_code", w2.Code)
+ }
+}
+
+// TestGetProfileHandler_ReadAny_ValidReasonCodeStillHitsTouchCap is the
+// PO review script's item 4: the specific regression handoff-03-auth.md
+// v3 fixed — a valid, logged reason_code satisfies Authorize() but does
+// NOT exempt the caller from the cumulative touch cap.
+func TestGetProfileHandler_ReadAny_ValidReasonCodeStillHitsTouchCap(t *testing.T) {
+ d := allowAllDeps()
+ d.TouchCounter = &fakeTouchCounter{allowed: false} // already over cap
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.getFn = func(ctx context.Context, id string) (*model.UserProfile, error) {
+ t.Fatal("Get must not be called once the touch cap is already exceeded, even with a valid reason_code")
+ return nil, nil
+ }
+ h := NewGetProfileHandler(d)
+ req := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req.URL.RawQuery = "reason_code=support_ticket"
+ req = req.WithContext(withScope("client1", ScopeReadAny))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusTooManyRequests {
+ t.Errorf("status = %d, want 429 — a valid reason_code must not exempt read:any from the cumulative cap", w.Code)
+ }
+}
+
+// TestGetProfileHandler_ScopeFailureAndAuthorizeDenial_IdenticalBody is
+// F7 (decisions/error-semantics.md): a scope failure and an authorize()
+// policy denial return an identical client-visible body — distinguished
+// only in the audit log, never in what the client sees.
+func TestGetProfileHandler_ScopeFailureAndAuthorizeDenial_IdenticalBody(t *testing.T) {
+ // Scope failure: caller has ScopeSearch but hits the read endpoint.
+ d1 := allowAllDeps()
+ h1 := NewGetProfileHandler(d1)
+ req1 := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req1 = req1.WithContext(withScope("client1", ScopeSearch))
+ w1 := httptest.NewRecorder()
+ h1(w1, req1)
+
+ // authorize() denial: right scope, policy says no.
+ d2 := allowAllDeps()
+ d2.Authz = &fakeAuthorizer{allow: false}
+ h2 := NewGetProfileHandler(d2)
+ req2 := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req2 = req2.WithContext(withScope("client1", ScopeReadOwn))
+ w2 := httptest.NewRecorder()
+ h2(w2, req2)
+
+ if w1.Code != http.StatusForbidden || w2.Code != http.StatusForbidden {
+ t.Fatalf("both must be 403, got %d and %d", w1.Code, w2.Code)
+ }
+ if w1.Body.String() != w2.Body.String() {
+ t.Errorf("scope failure body %q must be identical to authorize() denial body %q", w1.Body.String(), w2.Body.String())
+ }
+}
+
+func TestGetProfileHandler_MissingID_400(t *testing.T) {
+ d := allowAllDeps()
+ h := NewGetProfileHandler(d)
+ req := newGetRequest("")
+ req = req.WithContext(withScope("client1", ScopeReadOwn))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusBadRequest {
+ t.Errorf("status = %d, want 400", w.Code)
+ }
+}
+
+func TestGetProfileHandler_NotFound(t *testing.T) {
+ d := allowAllDeps()
+ repo := d.Repo.(*fakeRepository)
+ repo.profiles.getFn = func(ctx context.Context, id string) (*model.UserProfile, error) {
+ return nil, dao.ErrNotFound
+ }
+ h := NewGetProfileHandler(d)
+ req := newGetRequest("11111111-1111-1111-1111-111111111111")
+ req = req.WithContext(withScope("client1", ScopeReadOwn))
+ w := httptest.NewRecorder()
+ h(w, req)
+ if w.Code != http.StatusNotFound {
+ t.Errorf("status = %d, want 404", w.Code)
+ }
+}
diff --git a/internal/api/masking.go b/internal/api/masking.go
new file mode 100644
index 0000000..ac2cf6b
--- /dev/null
+++ b/internal/api/masking.go
@@ -0,0 +1,79 @@
+package api
+
+import (
+ "strings"
+
+ "loginid-takehome/internal/model"
+)
+
+// SearchResultProfile is profile:search's masked response shape —
+// handoff-03-auth.md v3: "masking is a profile:search-response property,
+// not a per-call flag." A distinct type (and distinct serialization
+// function, below) from the full read-response shape, per LT-39's own
+// acceptance criteria: "the search-response serialization path is a
+// distinct code path from the read-response one, not the same serializer
+// with a masking flag." Unmasked detail is obtained only through a
+// separate, separately-audited profile:read:own/profile:read:any call.
+type SearchResultProfile struct {
+ ID string `json:"id"`
+ Name string `json:"name"`
+ Phone *string `json:"phone,omitempty"` // masked, e.g. "***-***-1234"
+ Region *string `json:"region,omitempty"` // address reduced to locality/region only
+}
+
+// ProfileResponse is the full, unmasked shape returned by a
+// profile:read:own/profile:read:any call.
+type ProfileResponse struct {
+ ID string `json:"id"`
+ Name string `json:"name"`
+ Phone *string `json:"phone,omitempty"`
+ StreetAddress *string `json:"street_address,omitempty"`
+ Locality *string `json:"locality,omitempty"`
+ Region *string `json:"region,omitempty"`
+ PostalCode *string `json:"postal_code,omitempty"`
+ Country *string `json:"country,omitempty"`
+}
+
+// toSearchResult builds the masked search-response shape for one
+// profile. This function, not ProfileResponse's serialization, is the
+// only place a search result is ever produced — there is no shared
+// helper with a masking bool, by design.
+func toSearchResult(p model.UserProfile) SearchResultProfile {
+ return SearchResultProfile{
+ ID: p.ID,
+ Name: p.Name,
+ Phone: maskPhone(p.Phone),
+ Region: p.Region,
+ }
+}
+
+// toProfileResponse builds the full, unmasked read-response shape.
+func toProfileResponse(p *model.UserProfile) ProfileResponse {
+ return ProfileResponse{
+ ID: p.ID,
+ Name: p.Name,
+ Phone: p.Phone,
+ StreetAddress: p.StreetAddress,
+ Locality: p.Locality,
+ Region: p.Region,
+ PostalCode: p.PostalCode,
+ Country: p.Country,
+ }
+}
+
+// maskPhone reduces an E.164 phone to its last 4 digits, e.g.
+// "+15551234567" -> "***-***-4567" — nil stays nil (absent, not masked
+// into a value that looks like data).
+func maskPhone(phone *string) *string {
+ if phone == nil {
+ return nil
+ }
+ digits := *phone
+ if len(digits) < 4 {
+ masked := strings.Repeat("*", len(digits))
+ return &masked
+ }
+ last4 := digits[len(digits)-4:]
+ masked := "***-***-" + last4
+ return &masked
+}
diff --git a/internal/api/masking_test.go b/internal/api/masking_test.go
new file mode 100644
index 0000000..8d5a4c3
--- /dev/null
+++ b/internal/api/masking_test.go
@@ -0,0 +1,25 @@
+package api
+
+import "testing"
+
+func TestMaskPhone(t *testing.T) {
+ cases := []struct {
+ in *string
+ want *string
+ }{
+ {nil, nil},
+ {strp("+15551234567"), strp("***-***-4567")},
+ {strp("123"), strp("***")},
+ }
+ for _, tc := range cases {
+ got := maskPhone(tc.in)
+ switch {
+ case tc.want == nil && got != nil:
+ t.Errorf("maskPhone(%v) = %v, want nil", tc.in, *got)
+ case tc.want != nil && got == nil:
+ t.Errorf("maskPhone(%v) = nil, want %v", *tc.in, *tc.want)
+ case tc.want != nil && got != nil && *got != *tc.want:
+ t.Errorf("maskPhone(%v) = %v, want %v", *tc.in, *got, *tc.want)
+ }
+ }
+}
diff --git a/internal/api/ratelimit.go b/internal/api/ratelimit.go
new file mode 100644
index 0000000..6e1db7a
--- /dev/null
+++ b/internal/api/ratelimit.go
@@ -0,0 +1,62 @@
+package api
+
+import (
+ "context"
+ "sync"
+ "time"
+)
+
+// RateLimiter enforces handoff-03-auth.md v3's per-client-credential
+// rate limits (60 req/min for read:own/read:any, 10 req/min for
+// profile:search), keyed per sub+scope. The mechanism is this track's
+// call (v3, explicitly) — an interface so handler tests use a fake
+// instead of real wall-clock behavior.
+type RateLimiter interface {
+ // Allow reports whether sub may make one more request under scope
+ // right now, given limit requests per rolling minute.
+ Allow(ctx context.Context, sub string, scope Scope, limit int) (bool, error)
+}
+
+// InProcessRateLimiter is a fixed-window (per-wall-clock-minute) limiter,
+// in-process rather than Redis-backed — the simplest mechanism satisfying
+// v3's "the mechanism is your call" framing, sufficient for this
+// take-home's single-instance deployment. A fixed window (vs. sliding) can
+// admit up to 2x limit across a window boundary; accepted here as a
+// deliberate simplification, not hidden — a production multi-instance
+// deployment would need a shared store (Redis) instead.
+type InProcessRateLimiter struct {
+ mu sync.Mutex
+ buckets map[string]*bucket
+ now func() time.Time
+}
+
+type bucket struct {
+ windowStart time.Time
+ count int
+}
+
+// NewInProcessRateLimiter constructs a ready-to-use limiter.
+func NewInProcessRateLimiter() *InProcessRateLimiter {
+ return &InProcessRateLimiter{
+ buckets: make(map[string]*bucket),
+ now: time.Now,
+ }
+}
+
+func (r *InProcessRateLimiter) Allow(_ context.Context, sub string, scope Scope, limit int) (bool, error) {
+ r.mu.Lock()
+ defer r.mu.Unlock()
+
+ key := sub + "|" + string(scope)
+ now := r.now()
+ b, ok := r.buckets[key]
+ if !ok || now.Sub(b.windowStart) >= time.Minute {
+ b = &bucket{windowStart: now, count: 0}
+ r.buckets[key] = b
+ }
+ if b.count >= limit {
+ return false, nil
+ }
+ b.count++
+ return true, nil
+}
diff --git a/internal/api/ratelimit_test.go b/internal/api/ratelimit_test.go
new file mode 100644
index 0000000..9ce53aa
--- /dev/null
+++ b/internal/api/ratelimit_test.go
@@ -0,0 +1,65 @@
+package api
+
+import (
+ "context"
+ "testing"
+ "time"
+)
+
+func TestInProcessRateLimiter_AllowsUpToLimit(t *testing.T) {
+ rl := NewInProcessRateLimiter()
+ ctx := context.Background()
+ for i := 0; i < 3; i++ {
+ allow, err := rl.Allow(ctx, "clientA", ScopeSearch, 3)
+ if err != nil {
+ t.Fatalf("Allow: %v", err)
+ }
+ if !allow {
+ t.Fatalf("request %d denied, want allowed (limit 3)", i+1)
+ }
+ }
+ allow, err := rl.Allow(ctx, "clientA", ScopeSearch, 3)
+ if err != nil {
+ t.Fatalf("Allow: %v", err)
+ }
+ if allow {
+ t.Error("4th request within the same window should be denied at limit 3")
+ }
+}
+
+func TestInProcessRateLimiter_KeyedPerSubAndScope(t *testing.T) {
+ rl := NewInProcessRateLimiter()
+ ctx := context.Background()
+ if _, err := rl.Allow(ctx, "clientA", ScopeSearch, 1); err != nil {
+ t.Fatalf("Allow: %v", err)
+ }
+ // A different sub, or a different scope for the same sub, must have
+ // its own independent bucket.
+ allowOtherSub, _ := rl.Allow(ctx, "clientB", ScopeSearch, 1)
+ if !allowOtherSub {
+ t.Error("a different sub must not share clientA's bucket")
+ }
+ allowOtherScope, _ := rl.Allow(ctx, "clientA", ScopeReadOwn, 1)
+ if !allowOtherScope {
+ t.Error("a different scope for the same sub must not share the bucket")
+ }
+}
+
+func TestInProcessRateLimiter_WindowResets(t *testing.T) {
+ rl := NewInProcessRateLimiter()
+ fakeNow := time.Now()
+ rl.now = func() time.Time { return fakeNow }
+ ctx := context.Background()
+
+ if allow, _ := rl.Allow(ctx, "clientA", ScopeSearch, 1); !allow {
+ t.Fatal("first request should be allowed")
+ }
+ if allow, _ := rl.Allow(ctx, "clientA", ScopeSearch, 1); allow {
+ t.Fatal
… truncated (20 KB cap){
"command": "grep -n -i \"log\\b\\|logger\\|alert\\|audit\" /private/tmp/claude-501/-Users-warrenthompson-Source-LoginID-02-ai-security-architecture/85dbc0f6-04bb-4f99-a24d-158a68a2fd50/scratchpad/pr26.diff"
}271:+// audit log, never in what the client sees. Both call the one function 301:+// the caller should not count it as an alerting failure. 329:+ // ever, ***even in the log line a caller can't see*** — that 509:+// the audit log (not built by this story — see refinement/LT-39.md's 990:+// only in the audit log, never in what the client sees. 1065:+// separate, separately-audited profile:read:own/profile:read:any call. 1328:+ // alerting, not instead of it.
{
"command": "gh pr view 26 --repo wtvamp/loginid-takehome --json title,body,url 2>&1"
}{"body":"## Summary\n\nImplements `handoff-03-auth.md` v3's scope vocabulary and requirements for `profile:search` and `profile:read:own`/`profile:read:any`, per `refinement/LT-39.md`. Built and tested against fakes (`decisions/test-double-strategy.md`) — no auth middleware (S7/LT-40) and no real backend I/O in this story, both explicit non-goals.\n\n**Not wired into `cmd/api-service`'s live router yet.** Doing so before S7's JWT verification middleware exists would expose unauthenticated PII — exactly what the auth design exists to prevent. Wiring happens once S7 lands (refinement doc's \"Acceptance outcome\": handlers implemented and tested against fakes now, live-URL verification with a real token deferred, not waived).\n\n## Package layout (`internal/api`)\n\n- **`auth.go`** — `Scope`, `AuthContext` (+ context helpers), `ReasonCode` (closed enum), and `Authorizer` — the object-level policy hook (`authorize(sub, scope, target)`), called explicitly inside each handler after request parsing and before the DAO call, not as a second middleware, per v3's confirmed placement. Real implementation is a follow-up story against the token-issuer's own datastore (`caller_referral` lives there, per v3's F8 resolution) — this is the seam.\n- **`ratelimit.go` / `touchcounter.go`** — `RateLimiter` (60 req/min read:own/read:any, 10 req/min search) and `TouchCounter` (cumulative distinct-record-touch cap per caller per rolling 24h window, applying to `profile:read:any` unconditionally regardless of `reason_code` — the exact regression v3 fixed). Both interfaces with in-process implementations; a shared store (Redis) is a drop-in replacement for a multi-instance deployment. Crossing the touch cap hard-denies (429) subsequent requests for the remainder of the window — the request that crosses it still completes, only the next one is denied.\n- **`cursor.go`** — opaque pagination cursor at the API boundary, encoding the DAO's `Offset` underneath per v3's F-pag correction. Default page size 20, hard ceiling 50, enforced in the handler before the DAO is ever called.\n- **`masking.go`** — `profile:search`'s masked response (`SearchResultProfile`) is a distinct type with a distinct serialization function from the full read-response shape (`ProfileResponse`) — not the same serializer with a masking flag.\n- **`errors.go`** — boundary translation per `decisions/error-semantics.md`: only the DAO's seven sentinels' safe messages (plus `context.DeadlineExceeded` → 504) ever reach a client; everything else is a generic 500 with no wrapped detail. A scope failure and an `authorize()` denial return an identical client-visible body (F7) — enforced by both call sites routing through one function.\n- **`handlers.go`** — `NewSearchHandler` and `NewGetProfileHandler`. Search rejects an all-nil filter body with 400 before the DAO, and rejects a `page_size` over the cap outright rather than truncating. `GetProfile` handles both `profile:read:own` and `profile:read:any` behind one handler.\n\n## Tests\n\n`fakes_test.go` holds hand-written fakes for `dao.Repository`/sub-interfaces plus `Authorizer`/`RateLimiter`/`TouchCounter`, per `decisions/test-double-strategy.md`'s ruling. `handlers_test.go` covers the acceptance criteria directly, including the PO review script's specific regression tests:\n\n- item 4: a `profile:read:any` call with a **valid** `reason_code` still hits the cumulative cap\n- item 4a: a call made **after** the cap is crossed returns 429 and is **not served** — not merely observed to exceed the threshold\n- item 5: a scope failure vs. an `authorize()` denial produce byte-identical response bodies\n\n`cursor_test.go`, `masking_test.go`, `ratelimit_test.go`, `touchcounter_test.go` cover each collaborator directly.\n\n## Verification\n\n`go build`/`go vet` clean. `go test ./...` green (27 new tests in `internal/api`, full existing suite unaffected). `gofmt -l` clean.\n\n## Who did the work\n\n- **Renata Cole** (03 Engineering & Delivery, lead) — implementation and tests.\n- **Naomi Voss** (01, PO) — refinement.\n- **Desmond Okafor** (01, claims check) — verified the F-pag reading and every v3 cross-reference word-for-word.\n- **Tobias Lindqvist** (01, objection) — caught the cumulative-counter threshold-crossing ambiguity, routed to Marcus for the hard-deny ruling.\n- **Marcus Ilori** (02) — `handoff-03-auth.md` v3 and the hard-deny ruling.\n- Model: Claude Sonnet 5, via Claude Code.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)\n\nhttps://claude.ai/code/session_011Cuba4JUvrsQCrTdUj4yzk","title":"LT-39: REST handlers for search/retrieve user_profile (S6)","url":"https://github.com/wtvamp/loginid-takehome/pull/26"}{
"command": "grep -n -i \"audit\" handoff-03-auth.md 2>/dev/null | head -50"
}32:| `profile:read:any` | Retrieve a single record by id, no referral constraint | Search/query by fragment | Requires a `reason_code` from a closed enum (below) on every request, logged with the audit record | 41:**Response shape and masking (resolves F6 — 01's product requirement had no mechanism to bind to).** `profile:search` responses carry masked PII fields by default (e.g., a phone shown as `***-***-1234`, an address reduced to `locality`/`region` only) — unmasked, full-field detail is obtained only by a follow-up call under `profile:read:own` or `profile:read:any` against the specific record id returned by the search, which is its own separately-audited call per the audit-log section below. This uses scopes that already exist rather than inventing a fourth "expand" concept: masking is a `profile:search`-response property, not a per-call flag, and "tighter than search" is satisfied structurally because `read:own`/`read:any` are already narrower-provisioned scopes than `search`. 54:## Audit-log field list (every search/retrieve call)
{
"command": "sed -n '40,90p' handoff-03-auth.md 2>/dev/null"
}**Response shape and masking (resolves F6 — 01's product requirement had no mechanism to bind to).** `profile:search` responses carry masked PII fields by default (e.g., a phone shown as `***-***-1234`, an address reduced to `locality`/`region` only) — unmasked, full-field detail is obtained only by a follow-up call under `profile:read:own` or `profile:read:any` against the specific record id returned by the search, which is its own separately-audited call per the audit-log section below. This uses scopes that already exist rather than inventing a fourth "expand" concept: masking is a `profile:search`-response property, not a per-call flag, and "tighter than search" is satisfied structurally because `read:own`/`read:any` are already narrower-provisioned scopes than `search`. **Reject an empty search body before the DAO (resolves F10, first half).** A `profile:search` request with no `name` or `phone` filter at all is rejected by the handler with a 400 before it ever reaches the DAO — the DAO's own "an all-nil query is legal and returns every profile" behavior (`../05-data-ops/multi-db-strategy.md`) is a DAO-layer property this API must not expose directly. This is a shape-based rejection, distinct from and in addition to the count-based result-cap rejection above. ## Pagination, rate limits, decomposition resistance (requirements — numbers are fixed, mechanism is yours) - **Opaque cursor at the API boundary** (resolves F-pag — the earlier "cursor-based pagination only, no offset-based paging" was wrong to imply offset itself is the enumeration risk: a keyset cursor permits the same page-by-page walk, just statefully; the actual control is the cumulative counter below, and 05's DAO contract is `Offset int`, not keyset). The API-facing cursor may encode the DAO's offset directly — callers cannot construct or walk one by hand, which is the property the boundary exists to give you, and the DAO's offset is never caller-visible. **Enforcement site: the cursor-encoding/decoding step in the handler layer**, opaque to the caller regardless of what it encodes underneath. Default page size 20, hard ceiling 50 (05's DAO ceiling is 10,000 — this API's ceiling is the tighter, caller-facing one; don't expose the DAO's). - **Per-client-credential rate limits:** 60 req/min for `read:own`/`read:any`; 10 req/min for `profile:search`. **Enforcement site: rate-limiting middleware in `internal/api/`, keyed per client credential (`sub` claim).** - **Cumulative distinct-record-touch counter per caller per rolling 24h window**, tracked independently of per-minute rate — this is the control that actually resists decomposition-style enumeration (not the pagination scheme, per the F-pag correction above). **Applies to `profile:read:any` as well as `profile:search`, unconditionally on whether a `reason_code` is present** — a valid, logged `reason_code` satisfies the per-call `authorize()` policy check but does not exempt the caller from this cumulative cap (S8 review fix: the original wording made this counter a no-op for `read:any`, since that scope requires a `reason_code` on every call by design). **Enforcement site: a counting store (Redis, in-process with periodic flush, whatever fits your stack) incremented once per distinct record id returned, checked before the response is sent** — the mechanism is your call; the requirement is that it exists and applies to both scopes. **Ruling on threshold-crossing behavior (clarifying an ambiguity Tobias Lindqvist caught in LT-39 refinement — this was genuinely unclear as written, not a judgment call I'd already made and failed to state):** crossing the cumulative cap **hard-denies (429) further requests on that scope for the remainder of the rolling window**, in addition to alerting — not alert-only, not "log it and keep serving." An alert-only reading would make this counter a detection mechanism rather than the control S4/S8 built it to be, and would recreate exactly the "attribution stands in for volume control" gap the counter exists to close (Ingrid's S4 objection 4) — a caller who's crossed the cap and keeps getting served has, in practice, no cap. Both signals fire together: the 429 to the caller, and the alert to whoever monitors it, because a threshold that's silently enforced with no alert is just as bad operationally as one that alerts with no enforcement. - Sustained near-limit usage on any of the above should alert, not just silently throttle. **Enforcement site: the same rate-limiting middleware and counting store above, not a separate mechanism.** ## Audit-log field list (every search/retrieve call) Log these fields, and only these — see `handoff-04-secrets.md`'s never-log list for what must never appear here: - `sub` (caller identity) - `scope` used - record id(s) touched (opaque ids, not the PII values) - `reason_code`, when the scope is `profile:read:any` - policy decision (`allow` / `deny`, and if `deny`, whether it was a scope failure or an `authorize()` policy denial — these are distinct signals for detection) - timestamp - request outcome (success/failure/rate-limited) Never in this log: PII field values, full JWTs, full request/response bodies. Per `handoff-04-secrets.md`'s new requirement, this log stream itself needs read access distinct from and narrower than whatever can read Secrets — that's 04's mechanism, but don't let the log ship without asking whether that separation exists. ## What's not yours to decide (already decided upstream, don't re-derive) - **Grant type, token format, and claim set** — decided in `api-auth-design.md`; not open for re-litigation during implementation. - **Scope vocabulary and provisioning bars** — decided in S4 (`decisions/search-authz-scoping.md`); if a real implementation constraint makes one of these unworkable, flag it back to me rather than adjusting it unilaterally. - **The `DB_DSN_FILE` convention** — per `handoff-04-secrets.md`'s S7 revision, this track is asking you to add a file-based alternative to `DB_DSN` (env var becomes a transitional fallback, not a permanent equal option), because a DSN's embedded password is exposed by `kubectl describe`/crash dumps/child-process environment the same as any other secret in that inventory. This is a request into your config surface, not a unilateral change on my part — you decide the exact mechanism (a `_FILE` suffix convention or otherwise), I'm asserting the requirement. - **Password/client-secret hashing** — Argon2id, decided in `threat-model.md` Asset 1; not this hand-off's concern beyond noting it exists. - **PII retention/caching duration for anything the API returns** — `../05-data-ops/CLAUDE.md`'s call, not this track's or yours. - **Whether `read:any` ever gets issued to a real caller** — per S4, the scope stays defined but unissued unless a caller clears the provisioning bar (caller-role table entry, named approver). Don't wire a default grant "just in case." ## Open question back to you Nothing blocking. One thing worth a quick confirm when convenient: does your `internal/api/` middleware layer have a natural place to run the object-level `authorize()` check as a distinct step from scope validation (e.g., a second middleware or an explicit call inside each handler before the DAO call), or would you rather this track propose a specific Go interface shape for it? Either is fine — say which you'd find more useful and I'll adjust. --- Model: sonnet (Marcus Ilori, lead — compiled directly from S4's ruling, no new debate). Turns consumed: 0 hire turns (synthesis of already-ruled decisions per `PLAN.md` S6).
{
"command": "gh api repos/wtvamp/loginid-takehome/contents/refinement/LT-39.md --jq .content 2>/dev/null | base64 -d 2>/dev/null | head -100"
}(Bash completed with no output)
{
"command": "gh api repos/wtvamp/loginid-takehome/contents/refinement/LT-39.md --ref $(gh pr view 26 --repo wtvamp/loginid-takehome --json headRefName --jq .headRefName) 2>&1 | head -5\necho \"---\"\ngh pr view 26 --repo wtvamp/loginid-takehome --json headRefName,files --jq '.headRefName' 2>&1"
}unknown flag: --ref Usage: gh api <endpoint> [flags] Flags: --- feature/LT-39
{
"command": "gh api \"repos/wtvamp/loginid-takehome/contents/refinement/LT-39.md?ref=feature/LT-39\" --jq .content 2>&1 | base64 -d 2>&1 | head -150"
}base64: stdin: (null): error decoding base64 input stream
{
"command": "gh api \"repos/wtvamp/loginid-takehome/contents/refinement/LT-39.md?ref=feature/LT-39\" 2>&1 | head -30"
}{"message":"Not Found","documentation_url":"https://docs.github.com/rest/repos/contents#get-repository-content","status":"404"}gh: Not Found (HTTP 404){
"command": "gh api \"repos/wtvamp/loginid-takehome/git/trees/feature/LT-39?recursive=true\" --jq '.tree[].path' 2>&1 | grep -i -E \"LT-39|refinement\" "
}03-engineering-delivery/refinement 03-engineering-delivery/refinement/LT-32.md 03-engineering-delivery/refinement/LT-34.md 03-engineering-delivery/refinement/LT-35.md 03-engineering-delivery/refinement/LT-36-LT-37.md 03-engineering-delivery/refinement/LT-38.md 03-engineering-delivery/refinement/LT-39.md 04-infra-devops/refinement 04-infra-devops/refinement/LT-45-LT-47.md
{
"command": "gh api \"repos/wtvamp/loginid-takehome/contents/03-engineering-delivery/refinement/LT-39.md?ref=feature/LT-39\" --jq .content 2>&1 | tr -d '\\n' | base64 -d 2>&1"
}# LT-39 refinement — S6: REST handlers for search/retrieve user_profile (Q2) **Product Owner:** Naomi Voss (01). **Research team:** Imogen Hale, Desmond Okafor, Tobias Lindqvist. **Engineering:** Renata Cole (03). **Source:** `03-engineering-delivery/backlog.md` S6 (LT-39); `02-ai-security-architecture/handoff-03-auth.md` v3. **Who did the work:** Refined by Naomi Voss (PO); claims check Desmond Okafor; objection Tobias Lindqvist; engineering counterpart Renata Cole. ## Why this one now LT-32 (issuer Deployment split) is refined and handed off, unblocking S7 (auth middleware, LT-40), which this story sits alongside. LT-39 itself formally depends only on S2 (merged) and 02's accepted hand-off — it doesn't block on LT-32/S7 in `backlog.md`'s own dependency graph, and its handlers can be built and merged against fakes right now, per `decisions/test-double-strategy.md`. Refining it now means it's ready the moment S7 lands, rather than sitting as the next bottleneck after that. ## Acceptance outcome This is the first story that adds real, caller-visible REST behavior — not just a skeleton. But it depends on S7 (auth middleware) to be *truly* reviewable at the public URL with a real token; until S7 lands, "done" for this story specifically means: handlers implemented and tested against fakes, all criteria below satisfied in code and unit tests, with the actual live-URL, real-token exercise deferred to whenever S7 merges (same "verification pending" pattern as LT-36/LT-37 waiting on LT-38 — not waived, just sequenced). ## Acceptance criteria (restated from `backlog.md` S6, cross-checked against `handoff-03-auth.md` v3) - [ ] Implements `handoff-03-auth.md` v3's scope vocabulary (`profile:read:own`, `profile:read:any`, `profile:search`) and pagination/rate-limit numbers without re-deriving 02's authz-scoping reasoning. - [ ] `authorize(sub, scope, target)` runs as an explicit call inside each handler after request parsing, before the DAO call — not a second middleware, per v3's confirmed placement (it needs the parsed target id, which pre-routing middleware doesn't have). `authorize()` checks `caller_referral`/caller-role data against the token-issuer's own datastore, **not against 05's schema** — 05 was never assigned that table. - [ ] `profile:search` responses carry masked PII fields by default (phone as `***-***-1234`, address reduced to `locality`/`region` only). Unmasked detail requires a separate, separately-audited follow-up call under `profile:read:own` or `profile:read:any` against the specific record id — **this reuses the existing scopes, it does not invent a fourth "expand" flag**; masking is a `profile:search`-response property, not a per-call parameter. Enforcement site: the search-response serialization path is a distinct code path from the read-response one, not the same serializer with a masking flag. - [ ] **Pagination — clarified per v3's F-pag correction, since `backlog.md`'s original wording could be misread by an implementer:** the API-facing cursor is opaque to the caller — callers cannot construct or walk one by hand — but it may (and in the simplest correct implementation, does) encode the DAO's own `Offset` directly underneath. "Cursor-based pagination only" means **the caller-facing contract is opaque-cursor-only, not that the handler is forbidden from using the DAO's `Offset` field internally.** The actual anti-enumeration control is the cumulative distinct-record-touch counter (next bullet), not the cursor mechanism itself. Default page size 20, hard ceiling 50, enforced in the handler before the DAO is ever called — the DAO's own `Limit` bound (1..100, default 50) is a second, independent backstop only reachable if the handler clamp is bypassed, not a second policy a caller can rely on. - [ ] A cumulative distinct-record-touch counter per caller per rolling 24h window is enforced, independent of per-minute rate limits, applying to **both** `profile:search` and `profile:read:any` unconditionally — a valid `reason_code` on `read:any` satisfies the per-call `authorize()` check but does **not** exempt the caller from this cumulative cap (v3's explicit fix to an earlier wording that made the counter a no-op for `read:any`). Enforcement site: a counting store incremented once per distinct record id returned, checked before the response is sent. - [ ] **Resolved by 02's ruling (raised by Tobias's objection, routed to Marcus):** crossing the cumulative counter's threshold **hard-denies** (429) every subsequent `profile:search`/`profile:read:any` call from that caller for the remainder of the rolling 24h window — distinct from the 401/403 pair — **in addition to** alerting, not instead of it. An alert-only reading would make the counter detection rather than a control, recreating the "attribution stands in for volume control" gap 01's S4 objection closed at the product-requirements level. `handoff-03-auth.md` and `api-auth-design.md` updated with this explicit ruling. Enforcement site: the counting store's threshold check gates the request before it's served, returning 429; a separate alert fires regardless of whether the request was already being denied. - [ ] A search request with no filter field at all is rejected by this handler with a 400 before it reaches the DAO — the DAO itself treats an all-nil `ProfileQuery` as legal, so this handler-layer rejection is the only enforcement site for "no bare unconstrained search." - [ ] A `profile:search` query rejected for exceeding the result cap unconstrained (e.g. a bare wildcard) is rejected outright, not truncated — enforcement site: the same in-handler call site, checked before the query is built. - [ ] Unmapped DAO errors never surface `err.Error()` to the client; only the mapped sentinels' safe messages do — per `decisions/error-semantics.md`'s boundary translation function. Authn failures (missing/invalid token) return 401; scope failures and `authorize()` policy denials both return 403 with an identical client-visible body, distinguished only in the audit log. - [ ] `profile:read:any` requests carry a `reason_code` from the closed enum (`support_ticket`, `fraud_review`, `kyc_reverification`, `legal_hold`) on every call, logged with the audit record — adding a fifth value is a documented change to this enum, not a runtime config toggle. - [ ] When `api-service` calls `idp-connector` on a caller's behalf, it obtains a `connector:identity-lookup`-scoped token first, using its own `CONNECTOR_CLIENT_ID`/`CONNECTOR_CLIENT_SECRET` credential — distinct from S8's idp-connector-side handlers, which only validate that token, not issue or hold this credential. ## Non-goals - No auth middleware implementation — that's S7 (LT-40), a separate pull; this story's own tests use fakes for the auth context per `decisions/test-double-strategy.md`. - No real backend I/O — built against fakes, per `decisions/test-double-strategy.md`; LT-36/LT-37/LT-38 land separately. - No connector-side (`idp-connector`) handler work — that's S8, which only validates the token this story's handler obtains, not the same code. - No rate-limit/counter *mechanism* choice (Redis vs. in-process) — that's Renata's implementation call, per v3's own "the mechanism is your call" framing. ## PO review script 1. Confirm `authorize()` is called explicitly inside each handler, after parsing, before the DAO call — not registered as middleware. Read the handler code directly. 2. Confirm the search-response serializer and the read-response serializer are genuinely distinct code paths (grep for two separate serialization functions, not one with a masking conditional). 3. Confirm the pagination cursor is opaque at the API boundary (no caller-suppliable offset/page-number parameter accepted) — but don't fail the story if the handler internally passes `Offset` to the DAO; that's the correct implementation per the F-pag clarification above. 4. Confirm the cumulative distinct-record-touch counter's test coverage includes a `profile:read:any` case with a valid `reason_code` still hitting the cap — this is the specific regression v3 fixed, so it needs its own test, not just a `profile:search` test. 4a. Confirm a test exists asserting that a call made **after** the threshold is crossed returns 429 and is not served — not merely that the counter's value is observed to exceed the threshold — per 02's hard-deny ruling. 5. Confirm a 401/403 test pair exists asserting identical response bodies for a scope failure vs. an `authorize()` denial, distinguished only in a separate audit-log assertion. 6. Once S7 lands and this is deployed: a live-URL check with a real client-credentials token, confirming an actual masked search response and an actual 401 on a missing/expired token — deferred, not waived, per the Acceptance outcome above. ## Research team review **Desmond Okafor — claims check:** Verified the F-pag reading (opaque-at-the-boundary, not never-use-Offset-internally) word-for-word against v3. Verified the read-any/reason_code/cumulative-counter interaction, `authorize()` placement, caller_referral/F8 resolution, masked-response/F6 framing (confirmed as a near-verbatim quote, not editorializing), all-nil-search/F10, and bare-wildcard/F52 handling all match v3 exactly. Confirmed the reason_code enum values are a legitimate, sourced enrichment over backlog.md's vaguer original, not staleness. No corrections needed — cleanest refinement checked so far. **Tobias Lindqvist — objection:** The cumulative counter's "alerts on threshold, not just hard-blocks" wording doesn't clearly require denying the caller's subsequent requests once the threshold is crossed — as written, a caller over-cap could keep issuing served calls indefinitely with only an alert firing, recreating the "attribution stands in for volume control" gap his own S4 objection closed at the product-requirements level. Checked against the source: the ambiguity is genuinely in `handoff-03-auth.md` v3 itself, not a transcription error in this refinement — so this is 02's mechanism decision, not mine to invent. Routed to Marcus with Tobias's argument; the criterion is marked PENDING in the doc with his exact proposed wording ready to drop in if 02 confirms hard-deny. **Naomi Voss — ruling:** This refinement is accepted and ready to hand off in full. Marcus has since ruled on the pending item: crossing the cumulative counter's threshold hard-denies (429) subsequent calls, alerting in addition to, not instead of, the denial — exactly the outcome Tobias argued for. Updated above with his exact ruling and a matching PO-script step (4a). Every criterion is now resolved; nothing remains pending.
{
"command": "gh api \"repos/wtvamp/loginid-takehome/git/trees/feature/LT-39?recursive=true\" --jq '.tree[].path' 2>&1 | grep -i -E \"backlog|audit\""
}01-product-industry-research-design/backlog.md 01-product-industry-research-design/decisions/claims-audit.md 02-ai-security-architecture/backlog.md 03-engineering-delivery/backlog.md 04-infra-devops/backlog.md 05-data-ops/backlog.md
{
"command": "gh api \"repos/wtvamp/loginid-takehome/contents/03-engineering-delivery/backlog.md?ref=feature/LT-39\" --jq .content 2>&1 | tr -d '\\n' | base64 -d 2>&1 | grep -n -i \"audit\\|LT-39\\|LT-40\\|S6\\|S7\\|S8\" "
}96:## S6. REST handlers: search/retrieve user_profile (Q2) (LT-39) 103:- [ ] `profile:search` responses carry masked PII fields by default (e.g. phone as `***-***-1234`, address reduced to `locality`/`region` only); unmasked detail requires a separate, separately-audited follow-up call under `profile:read:own` or `profile:read:any` against the specific record id — enforcement site: the search-response serialization path, distinct from the read-response one (`handoff-03-auth.md` v3, resolves F6). 107:- [ ] Unmapped DAO errors never surface `err.Error()` to the client; only the mapped sentinels' safe messages do — enforcement site: the boundary translation function from `decisions/error-semantics.md`. Authn failures (missing/invalid token) return 401; scope failures and `authorize()` policy denials both return 403 with an identical client body, distinguished only in the audit log (`decisions/error-semantics.md`, consistency-pass F7). 108:- [ ] `profile:read:any` requests carry a `reason_code` from the closed enum on every call, logged with the audit record. 109:- [ ] When `api-service` calls `idp-connector` on a caller's behalf, it obtains a `connector:identity-lookup`-scoped token first, using its own `CONNECTOR_CLIENT_ID`/`CONNECTOR_CLIENT_SECRET` credential (`PLANNING.md` config surface) — enforcement site: this handler-side outbound-client code, distinct from S8's idp-connector-side handlers, which only validate that token, not issue or hold this credential. 118:## S7. Auth middleware (JWT validation) (LT-40) 138:## S8. Connector: /auth + /identity (Q3) (LT-41) 140:**Description:** `cmd/idp-connector`'s outbound client and inbound handlers for the assignment's fixed `/auth` and `/identity` contracts, per `connector-security.md`'s no-cache-by-default token lifecycle (fetch per `/identity` call, discard on return; fresh isolated `Authorization` header per outbound request, never inherited from pooling/keep-alive; fetch → use-once → zeroize, retries re-fetch via `/auth`). Includes the connector's own inbound self-auth (distinct audience `idp-connector-service`, scope `connector:identity-lookup`, restricted to `api-service` as caller) per `handoff-03-auth.md` v3 §"Where it's validated" (Ingrid's S8 catch), and the `context.WithTimeout` + bounded pool + backoff requirement from `decisions/go-layout-debate.md`. 158:**Description:** Table-driven unit tests for the handler layer (S6/S7, mocked DAO via S2's hand-written fakes) and the connector (S8, mocked HTTP), per `decisions/test-double-strategy.md`. The DAO's own cross-backend correctness is S5's job, not this story's — this covers request/response shape, auth rejection paths, and error-translation-to-HTTP-status mapping. 166:**Depends on:** S6, S7, S8. 201:**Depends on:** S2; **05: retention sweep contract addendum** (`multi-db-strategy.md` §3c, pending Oren's first-pass review — same process as the original contract); **04: Story 5 — Observability** (`04-infra-devops/backlog.md`, owns where the sweep job runs, and the `rows-examined`/`rows-deleted`/`oldest-surviving-row-age` metrics sink and alert rule this story's `OldestSurvivingAt` feeds). Named as Story 5, not Story 6 — Story 6 is log-sink retention for the audit trail, a different concern; Story 5 is the one that actually specifies the retention-sweep alerting this story depends on.
{
"command": "cat > /tmp/review26.md << 'EOF'\nIndependent review: Ingrid Solano (02 Designated Skeptic) — PR #26 (LT-39)\n\nChecked against `handoff-03-auth.md` v3 and the S4 ruling this implements, not just against the diff's own comments.\n\n**Confirmed correct (1, 3, 4, 6 of the review script):**\n- `authorize(sub, scope, target)` runs in-handler in both `NewSearchHandler` and `NewGetProfileHandler`, after request parsing (body decode / filter-shape check / cursor decode / reason_code validation) and before the DAO call, on every code path that reaches the DAO — no shortcut path found. For `profile:read:own`, this ordering also means an unauthorized caller gets 403 before the DAO ever runs, so existence of a record they can't reference never leaks through a 404-vs-403 distinction.\n- The masked/full split is a genuine type-and-function split (`SearchResultProfile` / `toSearchResult` vs. `ProfileResponse` / `toProfileResponse`) — `SearchResultProfile` structurally cannot carry `street_address`/`postal_code`/`country`, so there's no debug flag, content-type, or error path that could leak them; the field isn't there to leak.\n- `TouchCounter` counts distinct record IDs via a `map[string]struct{}`, not requests — verified by `TestInProcessTouchCounter_CountsDistinctRecordsOnly` and by the handler-level regression test that a valid `reason_code` on `read:any` still trips the cap. Crossing the cap hard-denies (429) the *next* request, matching the ruling exactly (not alert-only, not silent truncation).\n- Cursor is opaque (`v1:`-prefixed, base64) and the search request body has no offset/page field for a caller to pass directly — a caller who appends an unrecognized `\"offset\"` key to the JSON body has it silently ignored by `encoding/json`'s default behavior rather than rejected with a 400. No exploit path (the ignored field has no effect on the query), but it's a silent-ignore, not the \"rejected\" your script implies — worth a one-line note in the handler comment so a future reader doesn't assume decoder-level rejection is happening.\n\n**One real gap, not cosmetic (bears on item 2 and item 5 both):**\n\nThere is no audit-log emission anywhere in this PR — no interface, no call site, nothing. That's a problem specifically because LT-39's own acceptance criteria (not a later story's) require it twice: \"`reason_code`... logged with the audit record,\" and \"scope failures and `authorize()` policy denials both return 403 with an identical client body, **distinguished only in the audit log**.\" The PO review script's own item 5 asks the reviewer to \"confirm a 401/403 test pair exists asserting identical response bodies... distinguished only in a separate audit-log assertion\" — `TestGetProfileHandler_ScopeFailureAndAuthorizeDenial_IdenticalBody` proves the bodies are identical but has no audit-log assertion to prove alongside it, because there is nothing to assert against.\n\nThis isn't the same finding as \"the client can tell the two failures apart\" — the client genuinely can't, and that part is done right. The gap is that *nobody* can tell them apart right now, including whoever would monitor for repeated `authorize()` denials as a signal distinct from routine scope mismatches. The design's whole point in making the two failures look identical to the caller was to move that distinction into a channel the caller can't see, not to erase the distinction. As shipped, the distinction doesn't exist anywhere yet. Same root cause covers `reason_code`: it's validated and required, but never logged, so `profile:read:any` usage is currently unauditable in practice, not just \"the audit sink is someone else's mechanism\" (04's mechanism is the sink; the fields to write are this story's).\n\nGiven `internal/api` is explicitly the seam layer (auth middleware and DAO wiring both deferred to S7/live-wiring), an `AuditLogger` interface parallel to `Authorizer`/`RateLimiter`/`TouchCounter` — called at the same two sites `writeForbidden`/`writeUnauthorized` are called from, with a fake in tests the same way the other three collaborators have fakes — would close this without waiting on S7, and would let the PO script's item 5 actually be checked rather than partially checked.\n\n**What would change my mind:** if this is tracked as a separate, already-ticketed story (not \"the audit log in general is 04's problem,\" but specifically \"S6 emits these fields, ticketed as X\") that LT-39 was scoped to exclude, this is a sequencing note, not a defect — but as the PR and its own refinement doc read now, this looks like an acceptance criterion that shipped unmet, not one deferred on purpose.\n\nEOF\ncat /tmp/review26.md | wc -w"
}686
{
"command": "gh pr comment 26 --repo wtvamp/loginid-takehome --body-file /tmp/review26.md"
}https://github.com/wtvamp/loginid-takehome/pull/26#issuecomment-5651249543