feat: transform ChittyProof to dual-export library + service#2
feat: transform ChittyProof to dual-export library + service#2chitcommit wants to merge 12 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CHARTER.md: artifact type, HTTP endpoints table, expanded dependencies CHITTY.md: architecture, stack, endpoints, expanded consumers CLAUDE.md: full project structure, dual-export architecture, new commands Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the 📝 WalkthroughWalkthroughChittyProof transforms from a library-only project to a dual-export model with both SDK imports and HTTP service endpoints at proof.chitty.cc. The changes introduce Hono-based HTTP routes, Bearer token authentication middleware, comprehensive test coverage, and Cloudflare Worker configuration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
6-10:⚠️ Potential issue | 🟠 MajorExport surface is misaligned with the dual-export contract.
Line 6 points
maintosrc/worker.js, and Line 9 maps the package root import tosrc/lib/chittyproof-v2-canonical.js. This bypassessrc/index.js, so the documented SDK barrel isn't actually exposed from@chitty/chittyproof. Additionally, there is no./workerexport defined.Suggested manifest fix
- "main": "src/worker.js", + "main": "src/index.js", "exports": { ".": { - "import": "./src/lib/chittyproof-v2-canonical.js" + "import": "./src/index.js" }, + "./worker": { + "import": "./src/worker.js" + }, "./verify-ecdsa": { "import": "./src/lib/chittyproof-verify-ecdsa.js" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 6 - 10, The package manifest currently points "main" at src/worker.js and maps the package root import to src/lib/chittyproof-v2-canonical.js, which hides the SDK barrel in src/index.js and fails to export the worker; update package.json so the package root (".") import/require maps to ./src/index.js (exposing the documented SDK barrel), change "main" to point at src/index.js (or to remain compatible ensure it points to ./src/index.js), and add an explicit "./worker" export entry that maps to ./src/worker.js (include both "import" and "require" variants if you support ESM/CJS) so consumers can import the worker.
🧹 Nitpick comments (8)
src/routes/validate.js (1)
64-81: Add defensive checks for schema structure access.Accessing
schema.properties.fact.requiredetc. assumes the schema has this exact structure. If the schema file is malformed or missing these paths, this will throw at runtime.🛡️ Proposed defensive access
if (bundle.fact && typeof bundle.fact === "object") { - errors.push(...validateRequired(bundle.fact, schema.properties.fact.required, "fact")); + const factRequired = schema.properties?.fact?.required ?? []; + errors.push(...validateRequired(bundle.fact, factRequired, "fact")); errors.push(...validateConst(bundle.fact, "status", "sealed", "fact")); }Apply similar pattern for ledger, chain, score, and pillars sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/validate.js` around lines 64 - 81, The code assumes schema.properties and per-property entries exist; update the bundle validation blocks (the sections that call validateRequired and validateConst for bundle.fact, bundle.ledger, bundle.chain, bundle.score, bundle.pillars) to defensively read required arrays and const checks: first ensure schema && schema.properties && schema.properties.<prop> exist and that schema.properties.<prop>.required is an array (otherwise use an empty array), and only call validateConst for "status" when schema.properties.fact and its const expectation exist; pass the safe fallback arrays/values into validateRequired/validateConst to avoid runtime throws when the schema is malformed or missing paths.src/routes/verify.js (1)
33-40: Consider including ECDSA result even when hash verification fails.Currently, ECDSA verification is skipped when
hashResult.okis false. This is reasonable for performance, but consumers might want to know if the signature itself is structurally valid even when the hash doesn't match. Consider adding an option or documenting this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/verify.js` around lines 33 - 40, The current logic skips calling verifyECDSA when hashResult.ok is false, so result.ecdsa is omitted; update the flow in the handler around hashResult.ok and the ecdsa flag (the block that calls verifyECDSA(bundle, c.env) and sets result.ecdsa) to always perform verifyECDSA or make it conditional on a new option (e.g., includeEcdsaOnHashFail) so consumers can request ECDSA structural verification even if the hash fails; ensure verifyECDSA is invoked with the same parameters (bundle, c.env) and its return (ok, reason, kid) is assigned to result.ecdsa regardless of hashResult.ok when the option is enabled.src/middleware/auth.js (1)
19-21: Consider using timing-safe comparison for token validation.The direct
===comparison is susceptible to timing attacks. While exploitability is low for service-to-service auth, using a constant-time comparison is a security best practice for secrets.🔒 Proposed timing-safe comparison
+import { timingSafeEqual } from "node:crypto"; + +function safeCompare(a, b) { + if (typeof a !== "string" || typeof b !== "string") return false; + const bufA = Buffer.from(a); + const bufB = Buffer.from(b); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + export async function bearerAuth(c, next) { const expected = c.env.CHITTY_AUTH_SERVICE_TOKEN; if (!expected) { return c.json({ error: "unauthorized", message: "auth not configured" }, 401); } const header = c.req.header("authorization"); const token = header?.startsWith("Bearer ") ? header.slice(7) : null; - if (!token || token !== expected) { + if (!token || !safeCompare(token, expected)) { return c.json({ error: "unauthorized" }, 401); } await next(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middleware/auth.js` around lines 19 - 21, The token check in src/middleware/auth.js currently uses a plain equality (if (!token || token !== expected) ...) which is vulnerable to timing attacks; replace that branch with a timing-safe comparison using Node's crypto.timingSafeEqual (or an equivalent constant-time function): ensure both token and expected are non-null, convert them to Buffers of equal length (reject if lengths differ) and then call crypto.timingSafeEqual(bufferToken, bufferExpected); keep the early null/empty check and return c.json({ error: "unauthorized" }, 401) on mismatch.tests/worker.test.js (1)
26-48: Consider extending integration auth checks to the other protected endpoints.
/api/v1/canonicalize,/api/v1/hash, and/api/v1/validateare also guarded insrc/worker.js; adding 401/authorized-path checks here would harden end-to-end coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/worker.test.js` around lines 26 - 48, Add the same auth coverage you have for /api/v1/verify to the other protected routes by adding tests in tests/worker.test.js that call POST /api/v1/canonicalize, /api/v1/hash, and /api/v1/validate using the existing app.request helper and ENV: for each route add a request without an Authorization header expecting status 401, and a request with Authorization: "Bearer test-token-123" (and minimal valid JSON body similar to the verify test, e.g. { bundle: {}, ecdsa: false } or the route-appropriate payload) expecting a non-401 response (e.g. 400 for invalid payload), mirroring the pattern used in the "POST /api/v1/verify" tests.tests/routes/canonicalize.test.js (1)
25-34: Add an invalid-JSON test to cover the explicit parser error branch.The route has a dedicated
invalid JSON400 path, but this suite only covers missingpayload.✅ Suggested additional test
describe("POST /api/v1/canonicalize", () => { + it("returns 400 for invalid JSON", async () => { + const app = makeApp(); + const res = await app.request("/api/v1/canonicalize", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: "{invalid", + }); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toBe("bad_request"); + }); + it("returns 400 when payload is missing", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/canonicalize.test.js` around lines 25 - 34, Add a new test case in tests/routes/canonicalize.test.js to exercise the explicit "invalid JSON" parser error branch for the POST /api/v1/canonicalize route: create a test (e.g., it("returns 400 for invalid JSON payload", ...)) that uses makeApp(), sends a POST to "/api/v1/canonicalize" with header "Content-Type: application/json" but a malformed JSON body (e.g., "{invalid:") and asserts res.status is 400 (and optionally checks the error message/body). This will ensure the route's invalid-JSON handling is covered alongside the existing missing-payload test.tests/routes/verify.test.js (1)
13-59: Add one test that exercises the defaultecdsa=truebranch.Current tests force
ecdsa: false, so theverifyECDSAexecution path is untested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/verify.test.js` around lines 13 - 59, Add a test in tests/routes/verify.test.js that posts a valid bundle without the ecdsa flag so the server takes the default ecdsa=true path: create the bundle with makeFactProofBundle(), set bundle.signature.signed_fields_hash = await hashSignedPayload(bundle), call makeApp() and POST to "/api/v1/verify" with body JSON.stringify({ bundle }) (omit ecdsa), then assert res.status === 200, body.hash_ok === true and body.ecdsa === true to exercise the verifyECDSA branch.tests/routes/hash.test.js (1)
16-29: Use at least one fixed test vector instead of deriving expected output from the same implementation.Lines 16-28 compute
expectedHashviahashSignedPayload, which can mask shared regressions between route and library.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/hash.test.js` around lines 16 - 29, The test currently computes expectedHash by calling hashSignedPayload(bundle), which can hide regressions; replace that dynamic computation with at least one fixed test vector: hardcode a known good hash string (e.g., "sha256:<64-hex-chars>") for the given bundle payload, keep the request using makeApp() to POST to "/api/v1/hash" with the same bundle, and assert res.status is 200, body.signed_fields_hash equals the hardcoded value and still matches the /^sha256:[0-9a-f]{64}$/ regex; use the hashSignedPayload function only to generate the fixed value once externally (or from a trusted implementation) and embed that literal as expectedHash in the test so the test asserts against a fixed vector rather than deriving expected output from the same implementation.tests/routes/validate.test.js (1)
61-70: Assert the 400 error payload, not just status.On Line 69 you only assert
400. Add body assertions to lock the API contract (error+message) and catch regressions earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/validate.test.js` around lines 61 - 70, Update the "returns 400 when bundle is missing" test to assert the JSON error payload in addition to the status: after calling app.request("/api/v1/validate", ...) parse the response body (e.g., await res.json()) and assert it contains the expected error shape (an "error" field and a "message" field) and appropriate values/strings describing the missing bundle; keep the existing status check and use the same test block name to lock the API contract for the validate route.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHITTY.md`:
- Around line 47-55: The table starting with the header row "| Method | Path |
Auth | Purpose |" under the "### Endpoints" section (and the other table around
the second occurrence noted) needs surrounding blank lines to satisfy
markdownlint MD058; insert a single blank line immediately before the table and
a single blank line immediately after the table (so there is an empty line
between the preceding heading/paragraph and the table, and between the table and
the following content) for both tables to fix the lint warnings.
In `@CLAUDE.md`:
- Around line 85-86: The auth middleware's current token check uses plain
equality (token !== expected) which is vulnerable to timing attacks; update the
middleware (in src/middleware/auth.js, the auth middleware function handling
variables token and expected) to perform a constant-time comparison using
crypto.timingSafeEqual: import Node's crypto, convert token and expected to
Buffers, ensure they are the same length (if not, compare against a dummy buffer
of the same length to avoid short-circuit timing differences) and replace the
simple equality check with the timingSafeEqual-based check so unauthorized
responses still return the same way when the comparison fails.
In `@src/routes/validate.js`:
- Around line 10-20: The module currently uses readFileSync at top-level to
populate the schema variable (readFileSync + import.meta.url), which fails in
Cloudflare Workers; replace that file-system read with a direct import of the
exported schema module (the package export mapped to "./schema") or load it from
an asset binding/KV at runtime and assign to the schema variable. Locate the
schema variable and the readFileSync usage in this file (schema, readFileSync,
import.meta.url) and change to either: a static import of the JSON module or a
runtime fetch from the Worker asset/KV binding, ensuring schema is available
before validation code runs.
In `@src/worker.js`:
- Around line 22-24: The global error handler app.onError should not leak raw
exception messages or coerce every error to 400; change it to inspect the thrown
error (use instanceof HTTPException or check for a numeric err.status) and for
HTTPException return c.json({ error: "error", message: err.message },
err.status) otherwise return a generic 500 response like c.json({ error:
"internal_server_error", message: "Internal server error" }, 500); ensure
library errors are wrapped as HTTPException with appropriate status in the
places they are thrown (or caught and rethrown as HTTPException) and handle
expected client validation errors inside route handlers instead of relying on
the global handler.
In `@wrangler.toml`:
- Around line 20-22: The kv namespace entry uses a placeholder id ("TBD") which
will prevent deployment; create the KV namespace for the binding PROOF_KEY_CACHE
in your Cloudflare account (or retrieve the existing namespace ID) and replace
the id value under the [[kv_namespaces]] block so id is the actual namespace
GUID for PROOF_KEY_CACHE; ensure the binding name PROOF_KEY_CACHE matches the
code that references it.
- Around line 12-14: The TOML currently uses a single-table header [routes]
which is incorrect for Wrangler route bindings; replace the header with an
array-of-tables header [[routes]] (or use [route] if you intentionally want the
singular form) so the route entry (pattern = "proof.chitty.cc/*", zone_name =
"chitty.cc") is parsed correctly; update the header from [routes] to [[routes]]
to fix Wrangler config parsing.
---
Outside diff comments:
In `@package.json`:
- Around line 6-10: The package manifest currently points "main" at
src/worker.js and maps the package root import to
src/lib/chittyproof-v2-canonical.js, which hides the SDK barrel in src/index.js
and fails to export the worker; update package.json so the package root (".")
import/require maps to ./src/index.js (exposing the documented SDK barrel),
change "main" to point at src/index.js (or to remain compatible ensure it points
to ./src/index.js), and add an explicit "./worker" export entry that maps to
./src/worker.js (include both "import" and "require" variants if you support
ESM/CJS) so consumers can import the worker.
---
Nitpick comments:
In `@src/middleware/auth.js`:
- Around line 19-21: The token check in src/middleware/auth.js currently uses a
plain equality (if (!token || token !== expected) ...) which is vulnerable to
timing attacks; replace that branch with a timing-safe comparison using Node's
crypto.timingSafeEqual (or an equivalent constant-time function): ensure both
token and expected are non-null, convert them to Buffers of equal length (reject
if lengths differ) and then call crypto.timingSafeEqual(bufferToken,
bufferExpected); keep the early null/empty check and return c.json({ error:
"unauthorized" }, 401) on mismatch.
In `@src/routes/validate.js`:
- Around line 64-81: The code assumes schema.properties and per-property entries
exist; update the bundle validation blocks (the sections that call
validateRequired and validateConst for bundle.fact, bundle.ledger, bundle.chain,
bundle.score, bundle.pillars) to defensively read required arrays and const
checks: first ensure schema && schema.properties && schema.properties.<prop>
exist and that schema.properties.<prop>.required is an array (otherwise use an
empty array), and only call validateConst for "status" when
schema.properties.fact and its const expectation exist; pass the safe fallback
arrays/values into validateRequired/validateConst to avoid runtime throws when
the schema is malformed or missing paths.
In `@src/routes/verify.js`:
- Around line 33-40: The current logic skips calling verifyECDSA when
hashResult.ok is false, so result.ecdsa is omitted; update the flow in the
handler around hashResult.ok and the ecdsa flag (the block that calls
verifyECDSA(bundle, c.env) and sets result.ecdsa) to always perform verifyECDSA
or make it conditional on a new option (e.g., includeEcdsaOnHashFail) so
consumers can request ECDSA structural verification even if the hash fails;
ensure verifyECDSA is invoked with the same parameters (bundle, c.env) and its
return (ok, reason, kid) is assigned to result.ecdsa regardless of hashResult.ok
when the option is enabled.
In `@tests/routes/canonicalize.test.js`:
- Around line 25-34: Add a new test case in tests/routes/canonicalize.test.js to
exercise the explicit "invalid JSON" parser error branch for the POST
/api/v1/canonicalize route: create a test (e.g., it("returns 400 for invalid
JSON payload", ...)) that uses makeApp(), sends a POST to "/api/v1/canonicalize"
with header "Content-Type: application/json" but a malformed JSON body (e.g.,
"{invalid:") and asserts res.status is 400 (and optionally checks the error
message/body). This will ensure the route's invalid-JSON handling is covered
alongside the existing missing-payload test.
In `@tests/routes/hash.test.js`:
- Around line 16-29: The test currently computes expectedHash by calling
hashSignedPayload(bundle), which can hide regressions; replace that dynamic
computation with at least one fixed test vector: hardcode a known good hash
string (e.g., "sha256:<64-hex-chars>") for the given bundle payload, keep the
request using makeApp() to POST to "/api/v1/hash" with the same bundle, and
assert res.status is 200, body.signed_fields_hash equals the hardcoded value and
still matches the /^sha256:[0-9a-f]{64}$/ regex; use the hashSignedPayload
function only to generate the fixed value once externally (or from a trusted
implementation) and embed that literal as expectedHash in the test so the test
asserts against a fixed vector rather than deriving expected output from the
same implementation.
In `@tests/routes/validate.test.js`:
- Around line 61-70: Update the "returns 400 when bundle is missing" test to
assert the JSON error payload in addition to the status: after calling
app.request("/api/v1/validate", ...) parse the response body (e.g., await
res.json()) and assert it contains the expected error shape (an "error" field
and a "message" field) and appropriate values/strings describing the missing
bundle; keep the existing status check and use the same test block name to lock
the API contract for the validate route.
In `@tests/routes/verify.test.js`:
- Around line 13-59: Add a test in tests/routes/verify.test.js that posts a
valid bundle without the ecdsa flag so the server takes the default ecdsa=true
path: create the bundle with makeFactProofBundle(), set
bundle.signature.signed_fields_hash = await hashSignedPayload(bundle), call
makeApp() and POST to "/api/v1/verify" with body JSON.stringify({ bundle })
(omit ecdsa), then assert res.status === 200, body.hash_ok === true and
body.ecdsa === true to exercise the verifyECDSA branch.
In `@tests/worker.test.js`:
- Around line 26-48: Add the same auth coverage you have for /api/v1/verify to
the other protected routes by adding tests in tests/worker.test.js that call
POST /api/v1/canonicalize, /api/v1/hash, and /api/v1/validate using the existing
app.request helper and ENV: for each route add a request without an
Authorization header expecting status 401, and a request with Authorization:
"Bearer test-token-123" (and minimal valid JSON body similar to the verify test,
e.g. { bundle: {}, ecdsa: false } or the route-appropriate payload) expecting a
non-401 response (e.g. 400 for invalid payload), mirroring the pattern used in
the "POST /api/v1/verify" tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
CHARTER.mdCHITTY.mdCLAUDE.mdpackage.jsonsrc/index.jssrc/middleware/auth.jssrc/routes/canonicalize.jssrc/routes/hash.jssrc/routes/health.jssrc/routes/validate.jssrc/routes/verify.jssrc/worker.jstests/middleware/auth.test.jstests/routes/canonicalize.test.jstests/routes/hash.test.jstests/routes/health.test.jstests/routes/validate.test.jstests/routes/verify.test.jstests/worker.test.jswrangler.toml
| ### Endpoints | ||
| | Method | Path | Auth | Purpose | | ||
| |--------|------|------|---------| | ||
| | GET | `/health` | None | Health probe | | ||
| | GET | `/api/v1/status` | None | Service metadata | | ||
| | POST | `/api/v1/verify` | Bearer | Hash + ECDSA verification | | ||
| | POST | `/api/v1/canonicalize` | Bearer | Deterministic JSON | | ||
| | POST | `/api/v1/hash` | Bearer | SHA-256 of canonical payload | | ||
| | POST | `/api/v1/validate` | Bearer | FACT v2 schema validation | |
There was a problem hiding this comment.
Fix markdownlint MD058 around tables.
Line 48 and Line 75 start tables without required surrounding blank lines, which will keep lint noisy.
Suggested markdown fix
### Endpoints
+
| Method | Path | Auth | Purpose |
|--------|------|------|---------|
| GET | `/health` | None | Health probe |
| GET | `/api/v1/status` | None | Service metadata |
| POST | `/api/v1/verify` | Bearer | Hash + ECDSA verification |
| POST | `/api/v1/canonicalize` | Bearer | Deterministic JSON |
| POST | `/api/v1/hash` | Bearer | SHA-256 of canonical payload |
| POST | `/api/v1/validate` | Bearer | FACT v2 schema validation |
+
...
### Dependencies
+
| Service | Purpose |
|---------|---------|
| ChittyCert | JWKS public key hosting for signature verification |
| ChittyAuth | Shared-secret Bearer token validation |
| ChittyTrack | Log/trace aggregation (tail_consumers) |
| ChittyBeacon | Health monitoring (probes /health) |
+Also applies to: 74-81
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 48-48: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHITTY.md` around lines 47 - 55, The table starting with the header row "|
Method | Path | Auth | Purpose |" under the "### Endpoints" section (and the
other table around the second occurrence noted) needs surrounding blank lines to
satisfy markdownlint MD058; insert a single blank line immediately before the
table and a single blank line immediately after the table (so there is an empty
line between the preceding heading/paragraph and the table, and between the
table and the following content) for both tables to fix the lint warnings.
| - Routes are thin wrappers around library functions — no business logic in route handlers | ||
| - Auth middleware uses simple string comparison against `env.CHITTY_AUTH_SERVICE_TOKEN` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if the file exists
if [ -f "src/middleware/auth.js" ]; then
echo "=== Inspecting auth middleware implementation ==="
cat -n src/middleware/auth.js
else
echo "File src/middleware/auth.js not found. Searching for auth-related files..."
fd auth . -type f 2>/dev/null | head -20
fiRepository: chittyapps/chittyproof
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Search for token comparison patterns
echo "=== Searching for token comparison patterns ==="
rg -n 'CHITTY_AUTH_SERVICE_TOKEN|token.*===|Bearer|crypto' --type-list | head -5
rg -n 'CHITTY_AUTH_SERVICE_TOKEN|token.*===|Bearer' -i 2>/dev/null || echo "No matches found"Repository: chittyapps/chittyproof
Length of output: 9552
🏁 Script executed:
#!/bin/bash
# Check CLAUDE.md to confirm documentation
echo "=== Checking CLAUDE.md lines 85-86 ==="
sed -n '80,90p' CLAUDE.mdRepository: chittyapps/chittyproof
Length of output: 677
Use constant-time comparison for bearer-token validation.
The middleware at line 19 of src/middleware/auth.js uses token !== expected for secret validation. This simple string comparison is vulnerable to timing side-channel attacks that could leak information about token validity or length. Use crypto.timingSafeEqual() instead:
if (!token || !crypto.timingSafeEqual(Buffer.from(token), Buffer.from(expected))) {
return c.json({ error: "unauthorized" }, 401);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 85 - 86, The auth middleware's current token check
uses plain equality (token !== expected) which is vulnerable to timing attacks;
update the middleware (in src/middleware/auth.js, the auth middleware function
handling variables token and expected) to perform a constant-time comparison
using crypto.timingSafeEqual: import Node's crypto, convert token and expected
to Buffers, ensure they are the same length (if not, compare against a dummy
buffer of the same length to avoid short-circuit timing differences) and replace
the simple equality check with the timingSafeEqual-based check so unauthorized
responses still return the same way when the comparison fails.
| import { readFileSync } from "node:fs"; | ||
|
|
||
| const schema = JSON.parse( | ||
| readFileSync( | ||
| new URL( | ||
| "../../etc/authority/schema/chittyproof-v2-fact-bundle.schema.json", | ||
| import.meta.url, | ||
| ), | ||
| "utf8", | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List root files to understand project type
git ls-files | head -20Repository: chittyapps/chittyproof
Length of output: 626
🏁 Script executed:
#!/bin/bash
# Check for Cloudflare/Workers specific config
fd -e toml -e json -t f | grep -E "(wrangler|worker|cloudflare)" | head -10Repository: chittyapps/chittyproof
Length of output: 79
🏁 Script executed:
#!/bin/bash
# Verify schema file exists
find . -name "*chittyproof*" -o -name "*schema.json" | head -10Repository: chittyapps/chittyproof
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Check package.json for project type/scripts
find . -name "package.json" -type f | head -5Repository: chittyapps/chittyproof
Length of output: 80
🏁 Script executed:
#!/bin/bash
cat wrangler.tomlRepository: chittyapps/chittyproof
Length of output: 444
🏁 Script executed:
#!/bin/bash
cat package.jsonRepository: chittyapps/chittyproof
Length of output: 787
🏁 Script executed:
#!/bin/bash
cat src/routes/validate.jsRepository: chittyapps/chittyproof
Length of output: 2819
Replace readFileSync with imported schema or asset binding.
In Cloudflare Workers, using readFileSync with import.meta.url at module initialization is unreliable. The import.meta.url resolves to the bundled worker location at runtime, not the original filesystem, causing the relative path ../../etc/authority/schema/... to fail in production. Since package.json already exports the schema as a module export ("./schema": "..."), import the schema directly instead of reading it from the filesystem. Alternatively, use an asset binding or KV namespace to serve the schema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/validate.js` around lines 10 - 20, The module currently uses
readFileSync at top-level to populate the schema variable (readFileSync +
import.meta.url), which fails in Cloudflare Workers; replace that file-system
read with a direct import of the exported schema module (the package export
mapped to "./schema") or load it from an asset binding/KV at runtime and assign
to the schema variable. Locate the schema variable and the readFileSync usage in
this file (schema, readFileSync, import.meta.url) and change to either: a static
import of the JSON module or a runtime fetch from the Worker asset/KV binding,
ensuring schema is available before validation code runs.
| app.onError((err, c) => { | ||
| return c.json({ error: "bad_request", message: err.message }, 400); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if src/worker.js exists and examine the code around lines 22-24
git ls-files src/worker.jsRepository: chittyapps/chittyproof
Length of output: 79
🏁 Script executed:
# Read the src/worker.js file to examine lines 22-24
wc -l src/worker.js && echo "---" && cat -n src/worker.js | head -40Repository: chittyapps/chittyproof
Length of output: 1498
🏁 Script executed:
# Let's look at the routes to understand what errors might be thrown
fd -e js routes/Repository: chittyapps/chittyproof
Length of output: 48
🏁 Script executed:
# Let's check the directory structure and find route files
git ls-files src/routes/ | head -20Repository: chittyapps/chittyproof
Length of output: 176
🏁 Script executed:
# Let's examine one of the route files to see how errors are thrown
wc -l src/routes/verify.js && echo "---" && cat -n src/routes/verify.js | head -60Repository: chittyapps/chittyproof
Length of output: 1633
🏁 Script executed:
# Let's check the lib functions to see what errors they might throw
wc -l src/lib/chittyproof-v2-canonical.js && echo "---" && head -80 src/lib/chittyproof-v2-canonical.jsRepository: chittyapps/chittyproof
Length of output: 2252
🏁 Script executed:
# Search for throw statements in lib directory to see what errors can be thrown
rg "throw new Error" src/lib/Repository: chittyapps/chittyproof
Length of output: 845
🏁 Script executed:
# Let's check the verify-ecdsa file to understand those errors better
cat -n src/lib/chittyproof-verify-ecdsa.js | head -120Repository: chittyapps/chittyproof
Length of output: 4022
🏁 Script executed:
# Continue reading to find the "Unknown kid" error
cat -n src/lib/chittyproof-verify-ecdsa.js | sed -n '120,180p'Repository: chittyapps/chittyproof
Length of output: 2311
🌐 Web query:
Hono error handling err.status property support
💡 Result:
In Hono, app.onError((err, c) => ...) receives a generic error, so err.status is not guaranteed to exist (e.g., plain throw new Error() won’t have it). Hono’s docs show onError as an error hook that just gives you err and c, without promising HTTP fields like status. [1]
err.status is supported when the thrown error is a Hono HTTPException (and potentially by some ecosystem libraries that throw their own “HTTP-like” exceptions). Hono’s HTTPException is designed for this use-case: it carries a status and provides getResponse() to produce the corresponding Response. [2]
Typical pattern:
import { HTTPException } from 'hono/http-exception'
app.onError((err, c) => {
if (err instanceof HTTPException) {
// ok: err.status exists
return err.getResponse() // or: return c.text(err.message, err.status)
}
return c.text('Internal Server Error', 500)
})So: err.status is not a general Hono error feature; it’s specifically available when you throw/catch HTTPException (or another custom exception type that defines status). [2]
Sources:
[1] Hono app.onError API docs (error handling) (honojs.com)
[2] Hono HTTPException docs (status + getResponse()) (hono.ubitools.com)
Citations:
Do not expose raw exception messages or force all exceptions to 400.
Line 23 currently returns internal error text directly (e.g., "Unknown kid: xyz"), and lines 22–24 classify every thrown error as bad_request (400), including real server faults and internal errors. This weakens error semantics and can leak implementation details.
The suggested fix in the diff won't work as proposed because Hono's generic error handler doesn't guarantee err.status exists (it's only available for HTTPException). Instead, either:
- Wrap library errors in Hono's
HTTPExceptionwith appropriate status codes, or - Catch and handle expected client errors (400) in route handlers separately, letting unhandled errors default to 500.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/worker.js` around lines 22 - 24, The global error handler app.onError
should not leak raw exception messages or coerce every error to 400; change it
to inspect the thrown error (use instanceof HTTPException or check for a numeric
err.status) and for HTTPException return c.json({ error: "error", message:
err.message }, err.status) otherwise return a generic 500 response like c.json({
error: "internal_server_error", message: "Internal server error" }, 500); ensure
library errors are wrapped as HTTPException with appropriate status in the
places they are thrown (or caught and rethrown as HTTPException) and handle
expected client validation errors inside route handlers instead of relying on
the global handler.
| [routes] | ||
| pattern = "proof.chitty.cc/*" | ||
| zone_name = "chitty.cc" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Wrangler routes configuration syntax [[routes]] vs [routes] 2025
💡 Result:
In Wrangler (Cloudflare Workers) configs, routes is an array, so in TOML you define it as an array of tables:
-
Correct (multiple routes):
[[routes]] pattern = "subdomain.example.com/*" zone_name = "example.com" [[routes]] pattern = "subdomain-two.example.com/example" zone_id = "<YOUR_ZONE_ID>"
This matches the official docs’ TOML examples for
routes(array) using[[routes]]. [1], [2] -
Not correct for
routes:[routes]
In TOML,[routes]defines a single table, not an array, so it does not representroutes: Route[]as Wrangler documents it. Wrangler’s docs show[[routes]], not[routes], forroutes. [1], [2]
If you only need one route, Wrangler also supports the singular key:
-
Single route (simple string):
route = "example.com/*"
[1]
-
Single route (as an object/table):
[route] pattern = "example.org/*" zone_name = "example.org"
[3]
Sources: Cloudflare Wrangler configuration docs (routes/types) [1], Cloudflare Workers “Routes” doc (Wrangler examples) [2], sample Wrangler config showing [route] [3].
Change [routes] to [[routes]].
The Wrangler configuration uses [routes] which defines a single table in TOML, but Wrangler expects [[routes]] (array of tables) for route binding. Alternatively, if this is a single route, use [route] (singular) instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wrangler.toml` around lines 12 - 14, The TOML currently uses a single-table
header [routes] which is incorrect for Wrangler route bindings; replace the
header with an array-of-tables header [[routes]] (or use [route] if you
intentionally want the singular form) so the route entry (pattern =
"proof.chitty.cc/*", zone_name = "chitty.cc") is parsed correctly; update the
header from [routes] to [[routes]] to fix Wrangler config parsing.
| [[kv_namespaces]] | ||
| binding = "PROOF_KEY_CACHE" | ||
| id = "TBD" |
There was a problem hiding this comment.
KV namespace ID placeholder will block deployment.
The id = "TBD" placeholder must be replaced with a real KV namespace ID before deployment. Wrangler will fail to deploy with this value.
Per the PR checklist, ensure the PROOF_KEY_CACHE KV namespace is created and the actual ID is substituted here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wrangler.toml` around lines 20 - 22, The kv namespace entry uses a
placeholder id ("TBD") which will prevent deployment; create the KV namespace
for the binding PROOF_KEY_CACHE in your Cloudflare account (or retrieve the
existing namespace ID) and replace the id value under the [[kv_namespaces]]
block so id is the actual namespace GUID for PROOF_KEY_CACHE; ensure the binding
name PROOF_KEY_CACHE matches the code that references it.
Runs npm test on push to main and on pull requests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
1-26: LGTM! Clean and functional CI workflow.The workflow correctly:
- Triggers on push to
mainand pull requests targetingmain- Uses current action versions (
@v4)- Leverages npm caching for faster builds
- Uses
npm cifor reproducible installsOne optional enhancement to consider: adding a job timeout to prevent hung workflows from consuming runner minutes indefinitely.
💡 Optional: Add job timeout
jobs: test: runs-on: ubuntu-latest + timeout-minutes: 10 steps:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1 - 26, The CI job "test" currently has no timeout, so add a job-level timeout to prevent hung runs from consuming runner minutes; in the jobs.test definition add a timeout-minutes field (e.g., timeout-minutes: 60 or another suitable value) so the "test" job will be automatically canceled after the configured time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1-26: The CI job "test" currently has no timeout, so add a
job-level timeout to prevent hung runs from consuming runner minutes; in the
jobs.test definition add a timeout-minutes field (e.g., timeout-minutes: 60 or
another suitable value) so the "test" job will be automatically canceled after
the configured time.
1 participant
Summary
Transforms ChittyProof from a library-only package into a dual-export project: SDK barrel export (
src/index.js) for in-process consumers + Hono Worker (src/worker.js) deployed atproof.chitty.ccfor HTTP consumers.CHITTY_AUTH_SERVICE_TOKEN)/health,/api/v1/status,/api/v1/verify,/api/v1/canonicalize,/api/v1/hash,/api/v1/validateChanges
package.json— addedhono,wrangler, scripts (dev,deploy), main entrywrangler.toml— Worker config with KV binding, tail consumer, routesrc/index.js— SDK barrel re-exporting 13 functions from both lib modulessrc/worker.js— Hono app wiring routes + auth middlewaresrc/middleware/auth.js— Bearer token middlewaresrc/routes/— health, verify, canonicalize, hash, validate handlerstests/— 22 new tests (middleware, routes, integration)Pre-deployment steps
wrangler kv namespace create PROOF_KEY_CACHEwrangler secret put CHITTY_AUTH_SERVICE_TOKENwrangler secret put CHITTYCERT_JWKS_URL(fallback for cert.chitty.cc)register.chitty.ccTest plan
npm test)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores