fix/tariff-cov

PAIRS_UNACCOUNTED may append a second wildcard failure entry when one already exists

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts:246

resolveFinalizeCounters pushes a new wildcard entry (division:'*') when unaccounted > 0. If failures already contains a wildcard entry from a prior Catch (branch-level SFN error), the result has two division:'*' rows — one describing the branch error, one synthetic PAIRS_UNACCOUNTED. No data is lost but the operator-facing failure list is confusing.

droppedComponents type signature says non-nullable but null guard is present

domains/utility/src/tariff-rate/tariff-rate.scrape.ts:134

droppedComponents guards stored via (stored ?? {}), so null/undefined stored returns [] correctly for day-1 publishes. But the function signature shows stored: RateComponents (not nullable), creating an inconsistency between the type and the runtime guard that will confuse future readers.

tariff_jobs.result readable by any authenticated tenant — failure messages exposed

apps/platform/src/api/handlers/tariff-jobs.handler.ts:1

RLS policy tariff_jobs_select_authenticated grants SELECT on ALL columns to any authenticated user with USING (true) and no org scoping. The result.failures[].message column stores scrape failure text derived from CFE page content. sanitizePipelineError redacts the error column, but result column failure messages flow without sanitization. Tracked as BAT-320; not fixed in this branch.

503 dispatch path leaks raw AWS SDK/SFN error to the HTTP caller

apps/platform/src/api/handlers/tariff-jobs.handler.ts:108

When dispatchTariffJobPipeline fails (SFN StartExecution throws), the raw AWS SDK error message is passed directly to the 503 response body as { status: 'error', message: <raw SDK message> }. AWS SFN errors can contain ARNs, account IDs, and region details. sanitizePipelineError applies only to the DB-stored path, not this HTTP response path.

PROD_POSTGRES_URL is repository-scoped — any branch workflow can read it (BAT-316)

.github/workflows/tariff-coverage-watchdog.yml:57

The workflow adds a guard step ('Guard — non-default refs may only audit staging') but its own comment acknowledges this provides no real security boundary since the same branch can delete the step. The production DB credential is accessible to any branch that can run a workflow. Tracked as BAT-316; the real fix is scoping the secret to a 'Production' environment with protection rules.

rateTypes validated as z.string().min(1) — arbitrary strings reach URL lookup; closed vocabulary not enforced

packages/api/src/schemas/tariff-job.schemas.ts:39

The API schema accepts rateTypes: z.array(z.string().min(1)) without an enum constraint. The collector correctly guards with Object.hasOwn(RATE_PAGES, rateType) preventing exploitation, but arbitrary strings traverse handler → shell → DB → SFN input before that guard runs. The schema should enforce the closed vocabulary (GDMTH, PDBT, etc.) at the API boundary. A code comment acknowledges this gap.

echo ::error:: with DB-derived DETAIL is a GitHub Actions workflow command injection vector

.github/workflows/tariff-coverage-watchdog.yml:223

The final step emits echo "::error::${ENVN} tariff coverage: ${VERDICT} — ${DETAIL}". DETAIL contains tariff/zone codes from the database. If a code contained %0a::set-env:: or ::add-mask:: substrings, this echo would inject GH Actions workflow commands. The printf '%s\n' pattern used elsewhere in the file is the safe idiom; this echo path bypasses it.

CFE HTML page content flows unsanitized through SFN error.Cause into tariff_jobs.error before redaction

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:225

assertApplied quotes CFE page dropdown values verbatim into error messages. These become Lambda errorMessage → SFN error.Cause → transition Lambda → sanitizePipelineError. The sanitizer handles known infra patterns but arbitrary CFE-page-injected text (HTML, script content) survives truncation to 500 chars and is stored in tariff_jobs.error, which is world-readable (BAT-320). Risk is low (stored not reflected), but the data origin is an untrusted external HTTP response.

Collector Lambda has no outbound URL allowlist — relies solely on static rate-pages.json config

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:668

The collector fetches URLs exclusively from statically bundled RATE_PAGES config with no runtime URL prefix validation. If the bundled JSON were tampered with (supply-chain attack on the build artifact), arbitrary URLs could be fetched. An explicit startsWith('https://app.cfe.mx/') check would add defense-in-depth.

Watchdog DETAIL written to issue body without sanitization — Markdown injection risk

.github/workflows/tariff-coverage-watchdog.yml:180

"**Detail:** ${DETAIL}" is written to issue-body.md and posted as a GitHub issue body. DETAIL contains DB-derived tariff/zone codes. Markdown injection is possible if a tariff code contains Markdown syntax — low severity as GitHub issue Markdown injection is benign, but the pattern is inconsistent with the printf '%s\n' safe idiom used elsewhere.

startTariffJobShell: re-entry emits duplicate JOB_STARTED outbox event

domains/utility/src/tariff-job/tariff-job.shells.ts:165

decideStartJob returns ok({status:'running'}) when the job is already running, but StartTariffJobDecision has no alreadyRunning flag. The shell unconditionally calls update() and emit() after any ok() — so a re-delivered SFN MarkStarted invocation writes a second status='running' row and emits a second JOB_STARTED outbox event. finalizeTariffJobShell and failTariffJobShell both use alreadyFinalized/alreadyFailed flags to skip write+emit on re-entry. Fix: add alreadyRunning?: boolean to StartTariffJobDecision, set it to true in decideStartJob when job.status === 'running', and mirror the guard from finalizeTariffJobShell line 220.

500 status returned by mapper but absent from contract responses map

apps/platform/src/api/contracts/tariff-jobs.contract.ts:54

mapCreateTariffJobError returns status 500 for PersistenceError and the exhaustive-default branch, but the contract responses map declares only 201/401/403/409/422/503. ts-rest collapses undeclared status codes to a generic 500, masking the JSend error body. Add 500: JSendErrorSchema to the responses map.

persistScrapedRatesShell writes no outbox event despite bulk-mutating global pricing data

domains/utility/src/tariff-rate/tariff-rate.shells.ts:369

domain-patterns.md: 'All writes (entity + outbox) inside the same db.transaction() — non-negotiable'. persistScrapedRatesShell performs batchUpsert of tariff_rates inside a transaction but emits no outbox event. createTariffRateShell, updateTariffRateShell, deleteTariffRateShell all write outbox events. Downstream consumers of pricing data (billing, savings) receive no signal on a bulk rate refresh.

tariff-job.shells.ts omits createShellLogger structured logging

domains/utility/src/tariff-job/tariff-job.shells.ts:89

domain-patterns.md rule 82: shells must 'Use structured logging: createShellLogger(domain, operation, actor)'. tariff-rate.shells.ts correctly uses SharedFCIS.createShellLogger in every shell. tariff-job.shells.ts has no structured logging at all — start/finalize/fail/retry operations produce no log.start(), log.fail(), or log.success() calls. Operational tracing of SFN-invoked shells is impossible.

PersistScrapedRatesError defined inline in shells.ts, not in tariff-rate.errors.ts

domains/utility/src/tariff-rate/tariff-rate.shells.ts:280

Canonical form requires all error types and smart constructors in {entity}.errors.ts via TariffRateErrors.X() pattern. PersistScrapedRatesError is defined inline in shells.ts and constructed with inline literals. This bypasses the compiler-checked exhaustive switch pattern and makes errors ungrepppable from the canonical location.

Hardcoded 'tfj_' prefix literal in resolveJob instead of TARIFF_JOB_PREFIX constant

domains/utility/src/tariff-job/tariff-job.shells.ts:129

resolveJob uses jobOrId.startsWith('tfj_') to route between findByPublicId and findById. TARIFF_JOB_PREFIX = 'tfj' is already defined in @batu/database/schema. CLAUDE.md: 'never hardcode a prefix literal — derive from the runtime PublicIdPrefix'. Import TARIFF_JOB_PREFIX and use startsWith(TARIFF_JOB_PREFIX + '_').

tariff-jobs.contract.ts defines local JSend schemas instead of importing from @batu/api

apps/platform/src/api/contracts/tariff-jobs.contract.ts:19

The contract defines three local JSend schema helpers (JSendSuccessSchema, JSendFailSchema, JSendErrorSchema) duplicating canonical exports from packages/api/src/schemas/jsend.schemas.ts. The local JSendFailSchema uses open z.string() for the error field rather than the canonical registered ErrorCode enum. Error codes on this contract won't appear in the merged code registry or OpenAPI enum listing.

Integration tests run in zero CI gates — regressions in DB logic merge green (BAT-319)

.github/workflows/pr-checks.yml:83

43 *.integration.test.ts files run in no blocking CI gate, including the tariff-rate integration test for findEffectiveByTariffZoneAndPeriod — the baseline guard for the never-shrink rule. A regression in the cross-month period-resolution query can merge green. Tracked as BAT-319; not fixed in this branch. The blocking unit gate was strengthened here (good), but integration coverage gap remains.

Watchdog SQL test is textual only — query logic is never executed against Postgres in CI

scripts/ci/__tests__/tariff-coverage-watchdog-sql.test.ts:6

The file's own header states these are textual assertions on the SQL query, not execution. The critical round-trip scenarios (seed a gap → COVERAGE_GAP names it; add THOR pair → still ok) are described in a comment block but not written. The provider-scoping test only checks that a clause text appears twice, not that it filters THOR rows correctly. A one-character SQL mistake that makes the watchdog always-green would pass.

createTariffJobShell and startTariffJobShell have no shell-level tests

domains/utility/src/tariff-job/__tests__/tariff-job.shells.test.ts:1

tariff-job.shells.ts exports createTariffJobShell and startTariffJobShell, but the shells test only covers idempotency short-circuits in finalize and fail. Create (unique-constraint race) and start (already-running idempotency, which is also the high-severity duplicate-event bug) have no shell-level test. Per testing rules: 'Transaction atomicity, outbox events, query correctness' are shell-test territory.

Municipality cascade leg has no assertApplied check — implementation AND test gap

services/utility/tariffs/cfe/__tests__/collector.test.ts:350

The implementation calls assertApplied for division, state, month, and year but NOT municipality. The test suite mirrors this absence. A municipality no-op sets the wrong municipality context before division resolution, and since municipality IDs are only unique within a state, this could silently deliver a different division's rates. No test catches this because no test asks 'does the municipality leg get verified?'

tariff-job.shells.test.ts verifies outbox was called but not what event was written

domains/utility/src/tariff-job/__tests__/tariff-job.shells.test.ts:103

Shell tests verify emitDomainEvent was/wasn't called (call count) but never assert event type, aggregateId, or payload. A mutation that emits the wrong event type ('tariff_job.failed' instead of 'tariff_job.completed') or swaps the aggregateId would pass. Per testing.md: 'Verify outbox event was written with correct type, aggregateId, payload'.

persist.test.ts divisionsPersisted assertion uses > 0 instead of exact count

services/utility/tariffs/cfe/__tests__/persist.test.ts:333

The 'WARNED zone must not be subtracted' test asserts divisionsPersisted > 0, but the count formula (zones - skipped, excluding warned) is non-trivial. A mutation that subtracts warned zones would still yield > 0 if any zones were clean. The assertion should be exact (e.g., 3 zones, 1 warned → divisionsPersisted = 3).

transition.test.ts has no test for the handler() async bridge function

services/utility/tariffs/cfe/__tests__/transition.test.ts:386

All tested functions are pure exports. The async handler() — which calls reapStaleActiveJob, shells, and throws on missing job — has no test. Error paths (missing job throws, shell failure throws) and the 'finalize requires results or payload' guard are untested.

YEAR_MONTH_RE regex duplicated between domain decisions and transition Lambda

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts:37

YEAR_MONTH_RE is declared identically at tariff-transition.lambda.ts:37 and domains/utility/src/tariff-job/tariff-job.decisions.ts:72. The regex is non-trivial (month range guards 01-12); a drift between the two copies would silently accept different inputs in different contexts. Export from the domain decisions file and import in the Lambda, which already imports from '@batu/utility-domain'.

Mutable shared formState creates fragile implicit coupling in scrapeDivision

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:428

formState is a mutable Record captured by both the outer post() closure and scrapeDivision's delete calls. scrapeDivision deletes cascade keys to reset state before each division, but this relies on the outer post() mutating the same object. If scrapeDivision is ever extracted or a second caller is added, the coupling is invisible. Passing the accumulated form fields explicitly would remove the shared-mutable-state hazard.

collectedPairs Set can be replaced by a plain counter — duplicates are structurally impossible

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:639

collectedPairs tracks (rateType, division) pairs via a Set to compute divisionsCollected. Since each rate type scrapes a distinct page and divisions are unique within a page, no (rateType, division) duplicate can appear in rates[]. The Set adds allocation and tracking per entry for no correctness benefit; a simple counter would be clearer.

harvest() unconditionally fetches exit IP from api.ipify.org inside Chromium

services/utility/tariffs/cfe/src/lib/browser-clearance.ts:93

Every clearance run fetches exitIp from a third-party endpoint. The exitIp is captured for diagnostics but logged nowhere visible to the caller. This is an unnecessary external dependency per clearance run; consider making the fetch optional or removing it if the diagnostic value is low.