MARS-118 leg A: bearer-authed data-layer self-check route, with an allowlist that cannot silently shrink

Sub-tickets

Activity

• wi-cli-venus created · 5w ago
  parent=#1041
• wi-cli-venus decision · 5w ago
  DESIGN PING ANSWERED (coder-mars-cc). Both of coder02-mars-cc's calls decided; enforcement mechanism approved with three corrections, each measured rather than reasoned. CALL 1 - ENUMERATION SCOPE: GLOB, not a hand-listed module set. coder02's argument is correct and is the fail-closed polarity: a NEW module is covered by default and an exclusion is a reviewable diff line, whereas a hand-listed set has the same silent-decay hole one level up from the one this row exists to close. (Note the polarity: this is the OPPOSITE of the deny-list-ships-the-default hazard, because here the list being excluded SHRINKS coverage visibly instead of admitting an unlisted caller.) CORRECTION 1 - THE PROPOSED GLOB IS WRONG. 'src/lib/*.ts' assumes a flat directory. src/lib is NOT flat: src/lib/feedback/ holds feedback-core.ts, feedback-actions.ts, log-feedback.ts. Measured denominator: 130 non-test .ts files under src/lib, 3 of them invisible to the proposed glob. Use src/lib/**/*.ts minus *.test.ts. The test-file exclusion is NOT hygiene - src/lib/lint-no-silent-failure.test.ts:83 contains 'export async function f({ a }: { a: string })', a decoy fixture that would otherwise enter the enumeration and require a permanent EXEMPT line. CORRECTION 2 - THE PREDICATE HAS A LIVE HOLE, NOT A HYPOTHETICAL ONE. 'first parameter is a string named ...Id' misses the destructured-options signature, and that shape exists in the corpus today: users-actions.ts:167 listUsers({ and :981 mergeUsers({. The predicate must cover BOTH a positional ...Id: string first param AND a first param whose object type carries at least one ...Id: string property. Further, the test file must STATE ITS OWN COVERAGE BOUND - this enforces id-parameterised fetchers only; a list query taking no id, and any fetcher reachable only through a session-reading wrapper, are out of scope BY PREDICATE - and must PRINT the count of exported functions the predicate did not classify, so the denominator is visible in the test output instead of assumed away. CORRECTION 3 - THE FAIL-CLOSED ASSERTION NEEDS ONE MORE FLOOR AND MUST ITSELF BE EXERCISED. coder02 correctly identified that asserting each covered path resolves and yields at least one fetcher is what stops a moved/renamed module from making the enumeration pass vacuously on an empty corpus. It does not catch a glob that silently returns FEWER FILES after a refactor - the identical vacuous shape one level up - so also assert the corpus file count against a floor. And force the corpus assertion RED before trusting it (point the glob at a nonexistent directory, watch it fail), not only the added-fetcher case: an unexercised fail-closed branch is exactly the green line this control exists not to be. APPROVED AS WRITTEN: set equality in BOTH directions (an added fetcher goes red, and a renamed/deleted fetcher left in the allowlist ALSO goes red, so the map cannot rot into names that no longer exist), and a deliberate exemption as an explicit EXEMPT entry carrying a reason string in the test file. Also approved: the modules that call getSession() internally (administrar-actions, data-health-actions, postgrado-actions, self-service-assignment and the rest) land as EXCLUDED-with-reason - they are UNREACHABLE by the route, and recording them as exclusions rather than as coverage is the honest form. That set is MARS-721's subject matter. CALL 2 - THE BEARER: NEW VAR, SELFCHECK_API_KEY. Approved, and the blast-radius argument is stronger than coder02 put it: SUPPORT_API_KEY guards ops reply|note|close|reopen|notify-set, a MUTATION surface. One credential opening both a read-only probe and a ticket-mutation API is a far worse trade than one row of bookkeeping. Four binding conditions: (a) the mars-unverifiable-state.md row lands in the SAME change - coder-mars-cc owns that file, coder02 writes the row, reviewed in the diff; (b) the route FAILS CLOSED when the var is unset - 503, never 'no key configured therefore open'; (c) the key and its length are never echoed in any response or log; (d) response is no-store. STILL BINDING FROM THE PARENT DECISION: this is a query-layer control, it covers no rendering and no session-reading path, it does not close MARS-118, and MARS-118 stays blocked when this closes.
• wi-cli-venus spec · 5w ago
  DESIGN PING ANSWERED - BUILD SPEC SETTLED (coder02-mars-cc asked, coder-mars-cc approved with three corrections, 2026-09-11). Nothing built yet: the 213/553 push must land first, because gitpush's pre-push typecheck runs WHOLE-TREE and half-written route code in the tree would abort a push whose --only does not even list it. ENFORCEMENT IS BUILDABLE, so A is not decorative and the gating condition is met. Mechanism: a vitest test using the TypeScript compiler API (typescript ^5 is already a devDependency, vitest ^4.1.10 - no new dependency) that parses the covered modules, enumerates every EXPORTED function matching the fetcher predicate, and asserts SET EQUALITY against the route's allowlist keys. Both directions: an added fetcher goes red (the decay being gated), and a renamed or deleted fetcher left in the allowlist ALSO goes red, so the map cannot rot into names that no longer exist. A deliberate exemption is an explicit EXEMPT entry carrying a reason string in the test file - a reviewable diff line rather than an omission. CORPUS: GLOB, not a hand-listed module set - a new module would otherwise never be enumerated, which is the same decay one level up. CORRECTED GLOB (coder-mars-cc, measured): `src/lib/**/*.ts` minus `*.test.ts`. NOT `src/lib/*.ts` - src/lib is not flat, `src/lib/feedback/` holds 3 files (feedback-core, feedback-actions, log-feedback) that a single-star glob misses; denominator is 130 non-test .ts under src/lib. The test-file exclusion is not hygiene: `src/lib/lint-no-silent-failure.test.ts:83` contains a decoy `export async function f({ a }: { a: string })` fixture that would enter the enumeration and demand a permanent EXEMPT line. Modules that call getSession() internally (administrar-actions, data-health-actions, postgrado-actions, self-service-assignment and the rest) are UNREACHABLE by a bearer route and land as EXCLUDED-with-reason, never as coverage. PREDICATE, corrected - the original had a live hole. Cover BOTH shapes: (a) positional first parameter `...Id: string`; (b) first parameter whose OBJECT type has at least one `...Id: string` property - that destructured-options shape exists today (`users-actions.ts:167 listUsers({`, `:981 mergeUsers({`) and a positional-only predicate silently under-enumerates. The test file STATES ITS COVERAGE BOUND: this enforces id-parameterised fetchers only; a list query with no id, and any fetcher reachable only through a session-reading wrapper, are out of scope BY PREDICATE. It also PRINTS the count of exported functions the predicate did not classify, so the denominator is visible in the output instead of assumed away. FAIL-CLOSED, the load-bearing half, with the extra floor coder-mars-cc required: assert each covered path RESOLVES on disk AND yields >= 1 fetcher, AND assert the corpus FILE COUNT against a floor - a glob that silently returns fewer files after a move is the same vacuous-empty-corpus shape one level up, and it would read green forever. Acceptance of the enforcement itself requires forcing RED TWICE: once on the added-fetcher case (add a throwaway exported getFoo(fooId: string), watch it fail, revert) and once on the CORPUS assertion (point the glob at a nonexistent dir, watch it fail) - an unexercised fail-closed branch is exactly the green line this row exists not to build. CREDENTIAL: a NEW env var, SELFCHECK_API_KEY. Approved; NOT SUPPORT_API_KEY (src/app/api/internal/support-action/route.ts:65), which opens reply|note|close|reopen|notify-set - a MUTATION surface. One credential opening both a read-only probe and a ticket-mutation API is a far worse trade than one row of bookkeeping. CONDITIONS, all binding: the mars-unverifiable-state.md row lands in the SAME change (coder-mars-cc owns that file and reviews it in the diff); the route FAILS CLOSED when the var is unset - 503, never "no key configured, therefore open"; the key and its LENGTH are never echoed in any response or log; response is no-store. ROUTE SHAPE unchanged from the decision event: /api/internal/ path, Authorization: Bearer as the sole credential, X-Acting-Agent forensic-only, no user session minted or accepted anywhere. Representative ids resolved SERVER-SIDE by role, so there is no impersonation-shaped input. Response carries ONLY {name, ok, ms, errorName, errorMessage} - never row data. ROUTE ACCEPTANCE is the incident shape, not a generic throw: point one covered fetcher at a column that does not exist, confirm ok:false carries the SQL errorName, revert. LABEL, binding: query-layer control only. Does not cover rendering, does not cover any session-reading path, does NOT close the Pluto-incident class. MARS-720 closing does not unblock MARS-118.
• wi-cli-venus decision · 5w ago
  DESIGN AMENDED (coder-mars-cc), on coder02-mars-cc's measured objection. The single-allowlist enforcement approved earlier is REPLACED by a two-bucket form. coder02 prototyped the enumeration before writing it and found a hazard my gating condition would have turned into pressure - it was right to refuse to build and right to ask rather than widen the shape quietly. THE MEASUREMENT (coder02, TS compiler API over src/lib/**/*.ts minus *.test.ts): 130 corpus files, 423 exported function declarations, 65 matching the corrected predicate, 9 modules excluded for calling getSession internally carrying 40 more. THE HAZARD: the predicate keys on SIGNATURE, and a large share of the 65 matches are MUTATORS OR EMAIL SENDERS, not reads - sendPracticaReviewedEmail, sendPracticaComentarioEmail, sendPracticaToReviewEmail, sendPracticaJtpReassignedEmail, sendJtpBacklogDigest, sendWeeklyKpiOne, dispatchWelcomeEmail, createNotification, markAsRead, markAllAsRead, markComentarioNotificationsRead, deleteOrphanedApiImages, logOrphanedImages, emitRoleChange, createResumeLink, logRouteUsage, denyAccess. Under an allowlist-or-nothing rule, 'add it to the allowlist' is the cheapest way to clear a red build, and the allowlist is the set the ROUTE CALLS. A probe that sends a real email to a student, or flips notifications to read, is a strictly worse outcome than the uncovered read this row exists to cover - and it would arrive as someone innocently going green. APPROVED FORM: two buckets whose UNION must equal the enumeration exactly. ALLOWLIST = probed, read-only, what the route calls. NOT_PROBED = every other enumerated fetcher with a reason. Set equality is asserted against the union, so a new fetcher still fails the build, but the cheap path is now the SAFE bucket and putting a mutator in the probed bucket becomes a visible diff line. TWO ADDITIONS I REQUIRED, because the two-bucket fix RELOCATES the decay rather than removing it. (1) FLOOR THE ALLOWLIST, RATCHETED: with two buckets the set-equality assertion guarantees CLASSIFICATION, not COVERAGE - NOT_PROBED is the safe green default, so the allowlist can decay toward zero one honest-looking reason at a time with the build green the entire way. That is the same silent decay the gating condition exists to stop, moved one bucket over, and invisible for the same reason. Assert allowlist.length >= N with N committed in the test file; lowering N is then a visible diff line with a reason. Same instrument as the corpus file-count floor, one level up. (2) CLOSED VOCABULARY for NOT_PROBED reasons, not free text: free text admits TODO / not-yet / pending, which read as classification but are deferral and age into permanent. Fixed code set (sends-email, mutates, writes-archive, pure-no-query, session-bound; extensible at build time). An honest label costs one word; a dishonest one means picking a label a reviewer can see is wrong. Agreed unchanged: pure functions (resolveReviewerRelationAtReview, resolveComentarioRoleLabel) go to NOT_PROBED with pure-no-query rather than to the allowlist; per-bucket counts printed alongside the unclassified count. NOTED AS LOAD-BEARING: the predicate OVER-matches, and over-matching is the SAFE direction precisely because of the two-bucket design - an over-match costs a reason string, an under-match is silent. Keep it wide.
• wi-cli-venus progress · 5w ago
  SHIPPED AND ACCEPTED (coder02-mars-cc). Three pushes: 7a935ae v2.79.63 (the route, registry, probes, enforcement test, unverifiable-state row), 8b33456 v2.79.64 (temporary acceptance canary + SQLSTATE in the response + the note-must-name-its-path rule + this row's read-path correction), 3ce1283 v2.79.65 (canary removed). PASS:7a935ae deep from coder-mars-cc, no findings. One P3 from the drill remains in flight as a fourth small push (rail message), queued on venus-host.push-gate. ACCEPTANCE PASSED, and it is the incident shape rather than a generic throw. Run on 2.79.64 with a temporary probe whose query names a nonexistent column: probed=39, failed=1, and the failure carried errorCode 42703 with errorMessage 'column "thisColumnDoesNotExist" does not exist'. A query that is valid TypeScript and invalid against the LIVE SCHEMA surfaces through this route as an identified schema fault. errorName was "error" — exactly why the SQLSTATE field was added; without it the response could not distinguish drift from any other failure. SUBSTITUTION, APPROVED BEFORE IT RAN. The original criterion was to point a REAL covered fetcher at a nonexistent column, which means deploying a knowingly-broken query to a page real users load. A temporary module reachable only from the bearer route exercises the identical machinery (runProbe -> SELF_CHECK_PROBES[name] -> real query -> live schema -> 42703 in the response) with no user-visible breakage. coder-mars-cc approved and withdrew the original as the wrong ask. CLEAN BASELINE, the other half — a control only ever seen to FAIL is as uninformative as one only ever seen to pass. Run on 2.79.65: ok=true, probed=38, failed=0, 3,871 ms total; slowest getScopedStudents 1,131 / getScopedStudentKPIs 1,060 / getScopedPracticas 886 / isStudentInScope 811 / getPracticaChangelog 323; fastest 2 ms. The drill run was 2,287 ms with the same three scope queries at the top, so the spread is load rather than a change in the probe set. THIS IS THE FIRST REAL COVERAGE STATEMENT MARS HAS EVER HAD FOR ITS DATA LAYER: 38 reads executed against the live schema. Neither vitest nor the push gate can produce that sentence — the suite mocks @/lib/db and typegen emits Next route types, not DB types. PAGING DISCIPLINE. A failing probe logs error-tier, which pages the live PM and Elazar through the applog rail, so the drill WOULD page by design — and it was this route's first ever error-tier row. Announced on-channel immediately before the run (20:51 ART) and immediately after the revert deployed (20:56 ART), naming the window. coder-mars-cc confirmed the page was read as a drill and that he had the announcement in hand before it landed. An expected page arriving unannounced reads as a real incident, and once people learn some pages are drills they discount the next real one. SHAPE AS BUILT. POST /api/internal/self-check, bearer SELFCHECK_API_KEY only (fails CLOSED with 503 when unset, never "no key configured therefore open"), X-Acting-Agent forensic only, no-store, request body ignored entirely, ids resolved SERVER-SIDE so there is no impersonation-shaped input, response per probe { name, ok, ms, errorName, errorCode, errorMessage } and never row data. Bearer comparison hashes both sides to fixed-width sha256 digests so the key's LENGTH is not inferable by timing. Zero probes is a 503 failure, not an empty pass; a probe whose representative id is missing reports ok:false with ProbeContextMissingError. The response carries its own coverage sentence so a green body cannot be read as "the app is verified". THE GATING CONDITION, which was the actual deliverable. src/lib/self-check-registry.test.ts parses src/lib/**/*.ts with the TypeScript compiler API, enumerates every exported id-parameterised function (both the positional `...Id: string` and the destructured-options shape), and asserts set equality in BOTH directions against the registry: an added fetcher is red, and a renamed or deleted one left in the registry is red. Measured corpus 132 non-test modules, 423 exported functions in the non-session-bound ones, 65 matched. TWO BUCKETS, not one allowlist — a correction to the approved design and the most important decision on this row. The predicate matches on SIGNATURE and therefore over-matches: many of the 65 are MUTATORS and EMAIL SENDERS. With a single allowlist the cheapest way to turn a red build green would be adding one of those to the list the route CALLS — a probe that sends real student mail. So every match is classified PROBED (38) or NOT_PROBED (27, each with a reason from a CLOSED vocabulary: sends-email, mutates, writes-archive, pure-no-query, session-bound, needs-caller-data). Over-matching stays the safe direction; under-matching would be silent. FLOORS because set equality guarantees classification and not coverage: PROBED_FLOOR=38 and CORPUS_FILE_FLOOR=130 make any shrink a deliberate diff line. TWO PROBES REMOVED IN PRE-PUSH REVIEW, and the failure would NOT have been quiet. getAlumnoDetail and getDocenteDetail call requireRosterManager() -> requireCap("manageRoster") internally (persona-detail-actions.ts:144, :183), and requireCap REDIRECTS rather than throwing — a redirect() inside a route handler throws NEXT_REDIRECT, so each run would have reported a permanent false FAILURE or turned the whole response into a 307. Both are NOT_PROBED / session-bound. getUserArchiveTail in the same module does not call the guard and stays probed: the hazard is per-FUNCTION, not per-module. TRANSITIVE SESSION TAINT: computed, deliberately NOT enforced as an exclusion, on measurement. The value-import closure over src/lib taints 37 of 132 modules and would drop 8 probes, of which only those 2 are genuinely session-bound — profile-photo-nag's only auth.ts import is `import type { SessionUser }` (erased), reporting reaches auth.ts solely via hidden-users' pure normalizeEmail, exam-scores imports administrar-actions only for getComisionesForPeriodo (a bare SELECT at administrar-actions.ts:209). Module taint answers "does this FILE reach the session" when the question is "does this FUNCTION reach it", so enforcing it would have cut real coverage 15% to remove two bad probes AND still needed a human for the next one. Instead the taint FORCES a recorded human decision: every probed fetcher in a tainted module carries a dated note naming the path it verified (module -> module -> symbol, not an assertion of diligence), asserted in both directions so a note cannot outlive its subject. That reverse assertion is not theoretical — it failed on the first draft and caught a note written for an already-un-tainted module. EXHAUSTIVENESS IS ENFORCED BY tsc, not by the test: SELF_CHECK_PROBES is Record<ProbedFetcherName, Probe>, so a registry name with no probe is a type error and a probe with no registry entry is an excess-property error. It is also the only available shape — the probes module pulls "server-only" transitively and cannot be imported by vitest at all. THE ENFORCEMENT WAS FORCED RED FOUR TIMES, then restored: an unregistered fetcher added to a corpus module; CORPUS_FILE_FLOOR raised above the real count; the corpus dir pointed at a nonexistent path (loud ENOENT, no tests run); and the stale-note assertion, which failed on real data. A control never seen to fail is a green line, not a control. CREDENTIAL. SELFCHECK_API_KEY set production-only via the Vercel REST API (no local vercel CLI on venus), type=encrypted, value DM'd to the PM over llmmsg-srv and in no file, commit, memory note or WI. Deliberately NOT the support-action bearer, which opens a ticket-MUTATION surface. Per §No-Rotation neither lane rotates it; replacement is Elazar's call. CORRECTION TO THE UNVERIFIABLE-STATE ROW'S OWN READ PATH, measured at creation: GET /v9/projects/<id>/env returns a NON-NULL `value` for an encrypted variable, so "the API returned a value, therefore it is readable" is backwards and would stamp the entry verified. It is the CIPHERTEXT — 1168 characters against a 64-character key, sha256 of the two differ. Same class evolutiva-commons.md already rules on for Supabase Management API secret fields; coder-mars-cc is queuing a commons proposal naming Vercel env listings as a second instance. `last verified` applies to EXISTENCE and TARGET only. PTD. 7a935ae: live 2.79.63 == remote package.json 2.79.63, dpl_X7DmFCJFLMtznGMd71jE2CjDW4KX, build 39s zero error lines, no runtime-error clusters, both Class-A logs read despite the clean match. 8b33456: live 2.79.64, dpl_CMtMgggsUmYCxHGwF3E4H41siKRt. 3ce1283: live 2.79.65, dpl_C9bdfgkkaCVv64oEJqNbQaWT4G2u, remote package.json agrees in every case (gh api, never the local checkout). LABEL, unchanged and binding: this is a QUERY-LAYER control. It does not cover rendering, does not cover any session-reading path, and does NOT close the Pluto-incident class. MARS-720 DOES NOT TOUCH MARS-118's STATUS, which stays BLOCKED and is now deferred by pmmaster to after the cap reset. Published under these pushes, each reviewed on its own row BEFORE publication: bdedbc5 (155), 460ad02 (156), 4bab039 (157).
• wi-cli-venus completed · 5w ago
  CLOSED. Fourth and final push landed: f976f74 v2.79.66, live-version confirmed 2.79.66 against the REMOTE package.json on main (gh api, never the local checkout), dpl_3svyxFnKma5ZmgwSgkwmkGQcEefD, no runtime-error clusters in the hour covering the deploy. Depth artifact: DEPTH=deep CHECK=2c REASON="not provably inert: src/app/api/internal/self-check/route.ts" SAMPLED=no FILES=package.json + the route + self-check-message.ts + its test. Cold brief sent to coder-mars-cc for the cross-model review. THE CLOSING CONDITIONS, all three met: both acceptance runs recorded (drill on 2.79.64 probed=39 failed=1 errorCode 42703; clean baseline on 2.79.65 probed=38 failed=0 3,871 ms) - recorded in the previous event; the rail-line change landed at f976f74; and the formatter is exercised - by tests, which is the only thing that can exercise it. WHY THE LAST PUSH EXISTED, since it is the row's most transferable finding. The improved page line was built INLINE on the route's failure path, inside a logEvent(...).catch(() => {}). A throw in that expression - a missing field, an unexpected shape - would be swallowed by the catch and the page it exists to improve would become SILENCE, discovered by whoever is on the 3am drift failure. And the acceptance canary that could have executed that branch was deliberately removed at 3ce1283, so no deploy will ever run that code before the failure it reports. That is the shape: code that runs only inside a catch-all, on a path you have removed the ability to exercise, cannot be validated by shipping it - it must be made UNABLE to fail rather than observed not failing. Raised by coder-mars-cc as a P3, applying this row's own rule (a control never seen to fail is a green line) to the fix rather than to the feature. WHAT LANDED. formatProbeFailureSummary in src/lib/self-check-message.ts: every input field optional, every fallback total (name -> "unknown", errorCode -> errorName -> "?"), so a throw is unreachable by construction and not by inspection. SQLSTATE is preferred over errorName because errorName on a node-postgres error is just "error" and says nothing - the same measurement that put errorCode in the response. Six tests execute it directly: clean run; one failure with its SQLSTATE; SQLSTATE-over-errorName; the ProbeContextMissingError case with no driver code; the {} shape that would otherwise have swallowed the page; and the six-failure ellipsis, which also asserts the count stays the TRUE count - a truncated line understating the count would be worse than no line. 17 tests green in the two self-check files, 825/825 in the whole suite at the push gate, both typechecks clean (tsc --noEmit and tsc -p tsconfig.test.json - the first excludes *.test.ts and is green on them by construction). The 300-char MAX_ERROR_MESSAGE_LEN cap on probe error messages is untouched and still bounds LENGTH only, not content - the "BEFORE YOU ADD PROBE 41" note in self-check-registry.ts remains the binding constraint: a probe that passes an email address would turn a truncated driver error into PII disclosure. LABEL, unchanged and binding: QUERY-LAYER control. Covers no rendering and no session-reading path, does NOT close the Pluto-incident class. MARS-118 stays BLOCKED and deferred; this close does not touch its status.