fix: fail closed on missing review timestamps

[safal207]

Summary

Harden the trusted AI-review verifier so absent or malformed timestamp values fail closed.

This PR changes exactly one helper in one file:

• scripts/verify-ai-review-contract.cjs

Finding

Date.parse(value ?? 0) is unsafe for missing timestamps because Node/V8 may parse the coerced string "0" as a valid date instead of returning NaN. A missing created_at could therefore produce a nonzero freshness anchor and silently widen the accepted evidence window.

Correction

parseTime now:

1. returns 0 for non-string values;
2. returns 0 for empty or whitespace-only strings;
3. calls Date.parse only for a non-empty string;
4. preserves the existing finite-number guard.

All reviewer identities, request ordering, exact-head binding, submitted-review guards, status context, permission handling, retries, and timeout behavior remain unchanged.

Trust boundary

This patch must be merged into main before product PR #173 is updated. PR #173 will then:

• copy the exact verifier bytes from the new trusted merge;
• pin its workflow checkout to that immutable merge SHA;
• rerun exact-head CI and CodeRabbit review;
• resolve PRRT_kwDOS-8ZNM6OsEnk only after the trusted patch is active.

Scope

No product, CSS, service-worker, visual, generated, or integrity-manifest files are changed.

Checklist

• One-file trust patch.
• Missing timestamps return 0 before Date.parse.
• Existing evidence acceptance rules unchanged.
• Fresh exact-head CodeRabbit review complete.
• Exact-head CI green.
• Squash-merged with expected head SHA.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 294863e2-c168-4955-9b1f-1f3a9de772c7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Изменена функция parseTime в scripts/verify-ai-review-contract.cjs: добавлена проверка, что входное значение — непустая строка (после trim), иначе возвращается 0 до вызова Date.parse. Ранее выполнялся Date.parse(value ?? 0) без такой проверки.

Changes

Исправление parseTime

Layer / File(s)	Summary
Валидация входа перед Date.parse scripts/verify-ai-review-contract.cjs	Не-строки и пустые строки теперь возвращают 0 до вызова Date.parse, что меняет обработку null/undefined и нестроковых типов.

Estimated code review effort: 1 (Trivial) | ~3 минуты

Possibly related PRs

• safal207/robys-coffee-house-demo#174: Вводит тот же скрипт верификации и исходную логику parseTime, которую данный PR правит.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	Не хватает разделов Evidence, AI review и Solo maintainer decision из шаблона; описание изменений есть, но шаблон заполнен не полностью.	Добавьте Evidence с проверяемыми артефактами, блок AI review с @codex review и @jules review, а также раздел Solo maintainer decision при необходимости.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Название кратко и точно отражает главное изменение: ужесточение обработки отсутствующих timestamp'ов в verifier.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/fail-closed-review-timestamps

---

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

[safal207]

@coderabbitai review

Exact head: 50becc0

[qodo-code-review]

PR Summary by Qodo

Fail closed when AI-review timestamps are missing or blank

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Make missing/malformed review timestamps resolve to 0 to prevent widened trust windows.
• Avoid V8/Node parsing quirks from Date.parse(value ?? 0) by guarding inputs first.
• Preserve all existing verifier semantics aside from the safer timestamp parsing behavior.

Diagram

graph TD
A["GitHub review payload"] --> B["Verifier script"] --> C["parseTime()"] --> D{"Timestamp valid?"} --> E["Freshness check"] --> F{"Accept evidence?"}
D -- "no/empty" --> G["Reject (fail closed)"]
F -- "no" --> G
F -- "yes" --> H["Pass"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Strict ISO-8601 validation before parsing

• ➕ Eliminates Date.parse implementation quirks across runtimes
• ➕ Makes accepted timestamp formats explicit at the trust boundary
• ➖ More code and potential for rejecting legitimate variants
• ➖ Needs clear contract/spec for acceptable formats

2. Adopt Temporal (or a dedicated date parsing library)

• ➕ More deterministic parsing/handling than Date.parse
• ➕ Cleaner semantics around invalid inputs
• ➖ Adds dependency/runtime requirements (Temporal polyfill) or bundle weight
• ➖ Overkill for a single guard in a small verifier

Recommendation: The chosen approach (fail-closed guard for non-string/blank values before calling Date.parse) is the best fit for this verifier: it directly addresses the identified coercion hazard with minimal surface-area change and preserves existing acceptance rules. Consider stricter format validation only if the contract needs to guarantee a specific timestamp format across environments.

Files changed (1) +2 / -1

Bug fix (1) +2 / -1

verify-ai-review-contract.cjsFail-closed timestamp parsing guard in parseTime() +2/-1

Fail-closed timestamp parsing guard in parseTime()

• Replaces 'Date.parse(value ?? 0)' with explicit checks that return 0 for non-string, empty, or whitespace-only inputs. Only calls 'Date.parse' on a non-empty string and retains the existing finite-number guard to prevent NaN from propagating.

scripts/verify-ai-review-contract.cjs

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 175 --repo safal207/robys-coffee-house-demo --json headRefOid,title,files --jq '.headRefOid, .title'
echo "---"
gh api repos/safal207/robys-coffee-house-demo/pulls/175/files --jq '.[].filename'

Length of output: 502

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh api repos/safal207/robys-coffee-house-demo/contents/scripts/verify-ai-review-contract.cjs?ref=50becc089f1ba17f6fcc8cec0c5ee2260208ffe6 --jq '.content' | base64 -d > /tmp/verify.cjs
wc -l /tmp/verify.cjs
grep -n "parseTime" -A 15 /tmp/verify.cjs

Length of output: 1748

---

@safal207 Проверено на exact head 50becc089f1ba17f6fcc8cec0c5ee2260208ffe6. Изменён один файл, как заявлено: scripts/verify-ai-review-contract.cjs.

parseTime (строки 13–17):

function parseTime(value) {
  if (typeof value !== "string" || value.trim() === "") return 0;
  const parsed = Date.parse(value);
  return Number.isFinite(parsed) ? parsed : 0;
}

Соответствует всем пунктам correction:

• non-string → 0;
• пустая/whitespace-строка → 0;
• Date.parse вызывается только для непустой строки;
• finite-number guard сохранён.

Дефект с Date.parse(value ?? 0) устранён — коэрсия отсутствующего таймстампа больше не может дать ложный nonzero anchor. Остальная логика (createdTimeOf, submittedTimeOf, headUpdateAnchor <= 0 guard) не затронута. Регрессий не найдено.

✅ Action performed

Review finished.

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.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[github-actions]

🚦 Roby’s Lighthouse contract

Mobile

Metric	Current	Baseline	Delta	Status
Performance	0.99	1.00	-1.00%	✅
LCP	1803 ms	1746 ms	+3.25%	✅
TBT	3 ms	5 ms	-35.00%	✅
CLS	0.00	0.00	-0.00%	✅
FCP	1409 ms	—	—	✅
Speed Index	1409 ms	—	—	✅
Total JS	21 KB	21 KB	+0.11%	✅
Hero file	78 KB	78 KB	-0.15%	✅
Hero transfer	0 KB	—	—	✅
Hero request	0 ms	—	—	✅

Hard assertions: ✅ · Regression: ✅
· Public Lighthouse report

Desktop

Metric	Current	Baseline	Delta	Status
Performance	1.00	1.00	0.00%	✅
LCP	519 ms	460 ms	+12.88%	✅
TBT	0 ms	0 ms	0.00%	✅
CLS	0.00	0.00	0.00%	✅
FCP	377 ms	—	—	✅
Speed Index	457 ms	—	—	✅
Total JS	21 KB	21 KB	+0.11%	✅
Hero file	78 KB	78 KB	-0.15%	✅
Hero transfer	0 KB	—	—	✅
Hero request	0 ms	—	—	✅

Hard assertions: ✅ · Regression: ✅
· Public Lighthouse report

D.1 runs in observability mode. The final gate is enabled only after the reviewed baseline commit.

[safal207]

Trust-Update-Exception: BOOTSTRAP_NOT_ON_DEFAULT_BRANCH
Head: 50becc0

This one-file PR patches the trusted verifier itself. The current default-branch AI workflow is the legacy inline gate and cannot validate the new verifier bytes before they are merged. Independent exact-head CodeRabbit status is successful, no review threads are open, and all ordinary exact-head security, browser, visual, performance, runtime, traceability, reviewdog, and route-preflight checks are green. Product PR #173 remains blocked until it pins and executes the immutable merge SHA produced by this trust patch.

[safal207]

Head: 50becc0

Reviewer	Scope	Evidence
CodeRabbit	Mandatory independent exact-head review of the one-file trusted timestamp parser patch	E5 — successful exact-head status, no actionable threads
Qodo	Supplemental review of the fail-closed timestamp correction	clean/advisory
Codex	Native bot review unavailable on this connector surface; owner-authored output not counted	E3 advisory / not independent

Overall conclusion: READY_WITH_ADVISORY_GAPS

All ordinary exact-head CI is green. The remaining legacy AI workflow gap is explicitly classified as BOOTSTRAP_NOT_ON_DEFAULT_BRANCH: it cannot validate new trusted-verifier bytes until this patch is merged into main.

[safal207]

Proof-Depth-Seal: PDG-001
Head: 50becc0
Depth: D6

[safal207]

/merge-ready 50becc0