[cristim]

Summary

Adds regression tests for the fail-closed branches in the 4-eyes tier-2
(email) identity check when the purchase creator's account cannot be
resolved, closing the A16-001 gap described in #256.

Root cause

checkDifferentApprover's tier-2 (email) path resolves the creator's email
via GetUserEmailByID and must deny the approval when that lookup errors or
returns an empty address (internal/purchase/approvals.go:279-288). Neither
branch had a test: every stub in internal/purchase returned a real
address. The empty-address case is not hypothetical: the Postgres store
returns ("", nil) on pgx.ErrNoRows
(internal/config/store_postgres.go:1587-1588), so a deleted creator row
lands exactly there. Without a test on this branch, a refactor that drops or
inverts the creatorEmail == "" check would leave
strings.EqualFold("", actorEmail) false for every actor, silently passing
dual control for any execution whose creator was deleted, while the suite
stayed green.

Fix

Test-only change: added
TestManager_ApproveExecution_FourEyesOn_UnresolvableCreator_Denied in
internal/purchase/approvals_test.go, with two subtests on the
ApproveExecution (tier-2, email) call shape used by the SQS/token worker
path:

• GetUserEmailByID returning ("", nil) -- mirrors the real Postgres
  store's result for a deleted creator row
• GetUserEmailByID returning an error

Both assert the fail-closed error message and AssertNotCalled on
TransitionExecutionStatus. No production code changed; the guard was
already correct.

Regression test proof (mutation check)

Confirmed the new test has teeth by temporarily dropping the guard:

 	creatorEmail = strings.TrimSpace(creatorEmail)
-	if creatorEmail == "" {
-		logging.Warnf("purchase[%s]: 4-eyes mode on; creator account %s not found, denying (fail-closed)",
-			executionID, *execution.CreatedByUserID)
-		return fmt.Errorf("4-eyes approval mode is enabled but the creator's account could not be resolved; an admin must investigate before approving")
-	}
 
 	if strings.EqualFold(creatorEmail, actorEmail) {

With that mutation, the empty-address subtest panics on an unexpected
TransitionExecutionStatus mock call (the approval proceeds instead of
being denied) instead of asserting cleanly:

mock: I don't know what to return because the method call was unexpected.
	TransitionExecutionStatus(...)

Reverted the mutation and confirmed both subtests pass again (3 passed in 1 packages, including the existing SQS-actor-equals-creator test in the
same t.Run group).

Verification

All run with GOTOOLCHAIN=go1.26.6 GOWORK=off from the repo root:

• go build ./... - ok
• go vet ./internal/purchase/... - ok
• go test -race -short ./internal/purchase/... - 316 passed in 1 packages
• go mod tidy -diff - empty
• golangci-lint run --timeout=10m ./internal/purchase/... at the exact
  CI-pinned version (v2.10.1) - 0 issues

Closes #256

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 15 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available. Your 36 included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: a16d6b67-3031-4a22-b1af-864f2194a171

📥 Commits

Reviewing files that changed from the base of the PR and between 5bfa1db and ddc57e7.

📒 Files selected for processing (1)

• internal/purchase/approvals_test.go

---

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

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Independent review + local verification: MERGE at ddc57e7. Test-only; two mutants killed (empty-creator guard disabled; lookup error swallowed with a fabricated identity); both subtests mirror the real store contract ("", nil on ErrNoRows, and a DB error).