feat(broker): implement capabilities + policy + ledger primitives (5/7, signing deferred to #228)#240
feat(broker): implement capabilities + policy + ledger primitives (5/7, signing deferred to #228)#240chitcommit wants to merge 2 commits into
Conversation
…7, signing deferred to #228) Implements the broker-primitive REST surface specified in CHARTER.md "Git Broker Surface (REST, sensitive) — SPEC" (#235). Scope (Option A from audit — ships 5 of 7 primitives now): POST /api/v1/capabilities/mint — part of #209 POST /api/v1/capabilities/introspect — part of #209 POST /api/v1/capabilities/confirm — closes #210 (supersedes /api/git/confirm) POST /api/v1/policy/resolve — closes #211 POST /api/v1/ledger/emit — part of #209 (TODO: forward to chittyledger#11) Signing primitives (/api/v1/signing/sign-commit, /sign-tag) are deferred to #228 — #209 stays open until those land. Storage: - TOKEN_KV → live opaque capability + confirmation tokens (short-TTL, KV consistency with existing git-confirm pattern) - DB (D1) → tenant policy (018 + new 019 extensions) + broker audit trail Migration 019 extends 018 in place (ADD ONLY, no reshape): - git_tenant_policy — scalar per-tenant fields (force_push_allowed, default_author) that don't fit 018's tuple tables - git_protected_branches — protected_branches list per CHARTER §4 - broker_capability_audit — durable audit trail (also serves as the real-behavior ledger backing /ledger/emit until chittyledger#11 lands) Schemas (ajv): 10 schemas (input + output for all 5 primitives) under src/schemas/v1/. ChittyID pattern enforces canonical VV-G-LLL-SSSS-T-YYMM-C-XX per chittycanon://gov/governance. Deprecation: /api/git/confirm + /api/git/confirm/* return 410 Gone with replacement pointer (registered ABOVE auth so legacy callers get a clear signal without an API key). src/api/routes/git-confirm.js deleted. Tests: 13 integration tests via wrangler unstable_dev with REAL local D1 + KV — no mocks on DB, KV, or service modules. Migrations applied via `wrangler d1 execute DB --env=dev --local --file=...` in beforeAll; API key seeded into local API_KEYS KV for mcpAuthMiddleware. Test Files 1 passed (1) Tests 13 passed (13) Duration ~17s Validated: - All 10 ajv schemas compile + validate positive and negative cases - policy.resolve returns seeded chittyos-default tenant policy - capabilities.mint + introspect roundtrip with real KV - repo allowlist enforcement (POLICY_BLOCKED_REPO_NOT_ALLOWED) - schema-level enforcement of push requirements (ref + remote + confirmation) - protected-branch hard-deny (POLICY_BLOCKED_FORCE_TO_PROTECTED) on main - force_push_allowed=false denies even with valid confirmation token - ledger.emit operation/event_type binding (POLICY_BLOCKED_CAPABILITY_INVALID) - URL userinfo redaction in ledger payloads - legacy /api/git/confirm returns 410 + replacement pointer Closes #210 Closes #211 Partial of #209 (signing primitives blocked on #228) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyconnect | 2b4f4dd | Jun 04 2026, 01:55 PM |
|
Warning Review limit reached
More reviews will be available in 1 minute and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 332f9cf33b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Tenant policy disallows force-push", | ||
| ); | ||
| } | ||
| if (ref && policy.protected_branches.includes(ref)) { |
There was a problem hiding this comment.
For tenants that have force_push_allowed=1 but do not have explicit git_protected_branches rows for refs/heads/main and refs/heads/master (for example any newly added tenant, since the migration only seeds chittyos-default), this check lets a confirmed force capability be minted for the default branch. The removed legacy route hard-denied main/master independently of tenant data, and the new migration comment says missing branch rows should still honor those hard-deny defaults, so the mint path needs a built-in main/master guard as well as the DB list.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const repoOk = policy.repos.some((r) => prefixMatch(r.path_prefix, repo_path)); |
There was a problem hiding this comment.
When a tenant has a matching git_repo_allowlist row with access='read', this predicate still accepts it for commit, tag, and push operations because it only checks the path prefix. That lets a read-only repo allowlist grant write-capability tokens; the operation should require write/readwrite for write operations rather than treating every matching row as sufficient.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if (operation === "push") { | ||
| const remoteOk = policy.remotes.some((r) => globMatch(r.pattern, remote)); |
There was a problem hiding this comment.
This always applies glob matching and ignores the match_type column loaded from git_remote_allowlist. Rows configured with match_type='prefix' are valid per migration 018, but a pattern like https://github.com/CHITTYOS/ will not match https://github.com/CHITTYOS/repo.git here unless it also contains a wildcard, so legitimate tenant push policies using prefix matching are denied.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if (operation === "push") { | ||
| const remoteOk = policy.remotes.some((r) => globMatch(r.pattern, remote)); |
There was a problem hiding this comment.
This reduces the matching remote row to a boolean and then the force path only checks the tenant-wide force_push_allowed flag, so a tenant with global force enabled can mint a force capability for any allowed remote even when that row has allow_force=0. Migration 018 defines allow_force as the per-remote gate for force pushes, and the seeds default it to 0, so the mint logic should require the matched remote to allow force before accepting the confirmation token.
Useful? React with 👍 / 👎.
| * ledger emit. Closes chittyos/chittyconnect#210, #211. Partial of #209 | ||
| * (signing endpoints deferred to chittyos/chittyconnect#228). | ||
| */ | ||
| app.route("/api/v1", brokerPrimitivesRoutes); |
There was a problem hiding this comment.
Mounting brokerPrimitivesRoutes here also mounts its broker.use('*', mcpAuthMiddleware) wildcard under /api/v1, so requests to the existing secrets endpoints registered below this point (for example /api/v1/secrets/status, /rotate, and /upsert) now pass through MCP auth after their existing /api/v1/secrets/* authenticate middleware. In the Bearer/OAuth or Cloudflare-Access-only upsert contexts handled by authenticate, these requests have no X-ChittyOS-API-Key and are rejected before reaching the secrets handler, breaking the documented secrets API.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const repoOk = policy.repos.some((r) => prefixMatch(r.path_prefix, repo_path)); |
There was a problem hiding this comment.
Because this compares the raw request string, a repo path such as /home/ubuntu/projects/github.com/CHITTYOS/../outside/repo still starts with the seeded ChittyOS allowlist prefix even though it resolves outside that tree. Migration 018 says repo prefixes are canonicalized at check time; without normalizing and rejecting traversal before minting, the resource server can receive a valid capability for a path outside the tenant's allowed repo scope.
Useful? React with 👍 / 👎.
| } catch (e) { | ||
| console.error("[broker-audit] insert failed:", e.message); |
There was a problem hiding this comment.
When the D1 insert throws, this helper only logs the error and all callers continue returning success; for example /ledger/emit can return a ledger_event_id even though no durable broker_capability_audit row was written. On this sensitive broker surface the audit row is the durable ledger placeholder, so successful mint/confirm/introspect/ledger responses should fail closed or return an unavailable error when audit persistence fails.
Useful? React with 👍 / 👎.
| import { validate } from "../../schemas/v1/index.js"; | ||
|
|
||
| const broker = new Hono(); | ||
| broker.use("*", mcpAuthMiddleware); |
There was a problem hiding this comment.
The broker is wired to mcpAuthMiddleware, which only reads X-ChittyOS-API-Key; it ignores Authorization: Bearer even though the CHARTER authentication section allows bearer tokens and the existing API authenticate middleware supports that path. A resource server or client configured with bearer auth will get a 401 before reaching any of the new primitives, so this route should either use the shared auth middleware or extend MCP auth to honor bearer tokens.
Useful? React with 👍 / 👎.
|
|
||
| const { capability_token, event_type, payload } = body; | ||
|
|
||
| const capRaw = await c.env.TOKEN_KV.get(CAP_PREFIX + capability_token); |
There was a problem hiding this comment.
/capabilities/introspect rejects records whose expires_at is in the past, but /ledger/emit accepts any capability record still present in KV. If a record remains after its timestamp expires (for example in local persistent KV, manual repair, or delayed expiration), an expired capability can still emit a ledger event even though the broker's own error table classifies expired capabilities as invalid.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements the new Git broker REST surface under /api/v1/* (capability mint/introspect/confirm, policy resolve, ledger emit), adds AJV schemas + D1 migrations for policy/audit storage, adds integration tests against real local D1+KV, and deprecates legacy /api/git/confirm endpoints with 410 Gone responses.
Changes:
- Add
broker-primitivesroute module implementing 5/7 broker primitives (signing deferred). - Add v1 JSON Schemas (input/output) and AJV compile/validate helper.
- Add D1 migration
019for policy scalar fields, protected branches, and durable audit trail; add integration test suite; deprecate legacy endpoints.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
src/api/routes/broker-primitives.js |
New /api/v1/* broker primitives implementation (capabilities, policy, ledger emit) backed by KV + D1. |
src/index.js |
Mount broker primitives under /api/v1 and replace legacy git-confirm routes with unauthenticated 410 Gone. |
src/schemas/v1/index.js |
AJV compiler + exported validators for v1 broker schemas. |
src/schemas/v1/*.json |
JSON Schemas for broker primitive request/response envelopes. |
migrations/019_policy_resolve_extensions.sql |
Adds git_tenant_policy, git_protected_branches, and broker_capability_audit tables + seeds. |
tests/api/v1/broker-primitives.test.js |
Integration tests using wrangler unstable_dev with real local D1 + KV. |
package.json / package-lock.json |
Adds ajv + ajv-formats dependencies / lock updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const repoOk = policy.repos.some((r) => prefixMatch(r.path_prefix, repo_path)); | ||
| if (!repoOk) { | ||
| await auditEvent(c.env, { | ||
| event_type: "mint", | ||
| caller_chittyid, | ||
| tenant_id, | ||
| operation, | ||
| repo_path, | ||
| remote, | ||
| ref, | ||
| force_class, | ||
| outcome: "denied", | ||
| reason_code: "POLICY_BLOCKED_REPO_NOT_ALLOWED", | ||
| }); | ||
| return jerror( | ||
| c, | ||
| 403, | ||
| "POLICY_BLOCKED_REPO_NOT_ALLOWED", | ||
| "repo_path is not in tenant repo allowlist", | ||
| ); | ||
| } |
| if (operation === "push") { | ||
| const remoteOk = policy.remotes.some((r) => globMatch(r.pattern, remote)); | ||
| if (!remoteOk) { | ||
| await auditEvent(c.env, { | ||
| event_type: "mint", | ||
| caller_chittyid, | ||
| tenant_id, | ||
| operation, | ||
| repo_path, | ||
| remote, | ||
| ref, | ||
| force_class, | ||
| outcome: "denied", | ||
| reason_code: "POLICY_BLOCKED_REMOTE_NOT_ALLOWED", | ||
| }); | ||
| return jerror( | ||
| c, | ||
| 403, | ||
| "POLICY_BLOCKED_REMOTE_NOT_ALLOWED", | ||
| "Remote URL not in tenant allowlist", | ||
| ); | ||
| } | ||
| } |
| const ckey = CONFIRM_PREFIX + confirmation_token; | ||
| const craw = await c.env.TOKEN_KV.get(ckey); | ||
| if (!craw) { | ||
| return jerror( | ||
| c, | ||
| 403, | ||
| "POLICY_BLOCKED_CONFIRMATION_INVALID", | ||
| "Confirmation token missing or expired", | ||
| ); | ||
| } | ||
| let crec; | ||
| try { | ||
| crec = JSON.parse(craw); | ||
| } catch { | ||
| await c.env.TOKEN_KV.delete(ckey); | ||
| return jerror( | ||
| c, | ||
| 403, | ||
| "POLICY_BLOCKED_CONFIRMATION_INVALID", | ||
| "Confirmation token unreadable", | ||
| ); | ||
| } |
| ); | ||
| } | ||
|
|
||
| if (policy.protected_branches.includes(ref)) { |
| if (force_class !== "none") { | ||
| if (!policy.force_push_allowed) { | ||
| await auditEvent(c.env, { | ||
| event_type: "mint", | ||
| caller_chittyid, | ||
| tenant_id, | ||
| operation, | ||
| repo_path, | ||
| remote, | ||
| ref, | ||
| force_class, | ||
| outcome: "denied", | ||
| reason_code: "POLICY_BLOCKED_FORCE_TO_PROTECTED", | ||
| }); | ||
| return jerror( | ||
| c, | ||
| 403, | ||
| "POLICY_BLOCKED_FORCE_TO_PROTECTED", | ||
| "Tenant policy disallows force-push", | ||
| ); | ||
| } |
| ref, | ||
| confirmation_token, | ||
| } = body; | ||
| const force_class = body.force_class || "none"; |
| { | ||
| "if": { | ||
| "properties": { "force_class": { "enum": ["force", "force_with_lease"] } }, | ||
| "required": ["force_class"] | ||
| }, | ||
| "then": { "required": ["confirmation_token"] } | ||
| } |
| function redactPayload(obj) { | ||
| if (obj === null || typeof obj !== "object") return redactString(obj); | ||
| if (Array.isArray(obj)) return obj.map(redactPayload); | ||
| const out = {}; | ||
| for (const k of Object.keys(obj)) out[k] = redactPayload(obj[k]); | ||
| return out; | ||
| } |
| ledger_event_id: ledgerEventId, | ||
| domain: "git", | ||
| event_type, | ||
| scope: capRec.scope, |
| remote: capRec.scope.remote, | ||
| ref: capRec.scope.ref, |
chitcommit
left a comment
chitcommit
left a comment
There was a problem hiding this comment.
Review (request-changes equivalent — bot cannot self-request-changes)
Multi-reviewer pass (code-reviewer / silent-failure-hunter / pr-test-analyzer / comment-analyzer). 6 critical, 3 important, 3 doc-rot findings. Inline annotations below.
Critical
- Hard-deny of
main/masterregressed to be tenant-config dependent — tenants without explicitgit_protected_branchesrows can force-push to main; bare names (norefs/heads/prefix) never blocked. - Expired-token check fails open on missing/corrupt
expires_at(NaN < Date.now() === false). auditEventswallows D1 failures withconsole.erroronly —okmint/confirm/ledger_emit can issue tokens while the durable audit row silently fails./policy/resolveschema requirescaller_chittyid+operationbut handler reads onlytenant_id— any authenticated caller can dump any tenant's policy bundle. No audit row emitted./capabilities/confirmonly checks protected_branches; ignorespolicy.force_push_allowed— a tenant with force globally disabled can still mint confirmation tokens.- Test gap: no redeem-with-mismatched-scope test (#235 fix not covered), no cross-tenant replay test (#210 reopen reason), no expired-token introspect test. Several existing tests are ajv-only where behavior matters.
Important
git_remote_allowlist.match_typeselected but ignored — every entry silently treated as glob.- Migration 019 audit enum allows
expired/invalidbut no code path writes them. - KV
puton mint/confirm not wrapped — silent persistence failure possible.
Doc rot
- TODO references
chittyledger#11(a merged Dependabot PR, not a tracking issue). src/index.js:1551cites #228 for signing-deferral; #228 is the credentials issue. Signing-deferral is #242.- ChittyID schema description says YYMM, regex enforces
[0-9]{4}, canon (chittycanon://gov/governance) says YM. Description and regex agree; canon disagrees with both. Recommend follow-up issue rather than silent edit.
Fixes incoming on the same branch.
— attribution: code-reviewer (1,3,5,7,9), silent-failure-hunter (2,3,8,9), pr-test-analyzer (6), comment-analyzer (10,11,12), shared (4).
| "Tenant policy disallows force-push", | ||
| ); | ||
| } | ||
| if (ref && policy.protected_branches.includes(ref)) { |
There was a problem hiding this comment.
CRITICAL #1 (code-reviewer + silent-failure-hunter) — policy.protected_branches.includes(ref) is now tenant-config dependent. The chittyos-default seed includes refs/heads/main and refs/heads/master, but any other tenant without explicit rows in git_protected_branches can force-push to main. Additionally, bare main/master (no refs/heads/ prefix) are never blocked even for the seeded tenant. CHARTER says "Force-push to main/master is hard-denied" — that must be a universal constant, not data-driven. Fix: union per-tenant rows with universal set ["main","master","refs/heads/main","refs/heads/master"] in loadTenantPolicy. Apply to both mint and confirm code paths.
| return c.json({ active: false }); | ||
| } | ||
|
|
||
| if (new Date(rec.expires_at).getTime() < Date.now()) { |
There was a problem hiding this comment.
CRITICAL #2 (silent-failure-hunter) — Fails open on bad/missing expires_at. new Date(undefined).getTime() returns NaN; NaN < Date.now() is false, so a record with a corrupt or missing expires_at is treated as active and the caller's scope is returned. Validate that parsed expires_at is a finite number; treat NaN/missing as expired (active:false).
| ) | ||
| .run(); | ||
| } catch (e) { | ||
| console.error("[broker-audit] insert failed:", e.message); |
There was a problem hiding this comment.
CRITICAL #3 (silent-failure-hunter) — auditEvent catches D1 errors with console.error only. On outcome:"ok" for mint/confirm/ledger_emit, callers receive issued tokens or ledger ids even when the durable audit row silently fails — breaking the audit-before-issue guarantee. Refactor: throw on D1 failure for success paths; route handlers should return 503 AUDIT_WRITE_FAILED. Keep best-effort for denied/expired/invalid outcomes with tagged console.error.
| protected_branches: policy.protected_branches, | ||
| ...(policy.default_author ? { default_author: policy.default_author } : {}), | ||
| }; | ||
| return c.json(out); |
There was a problem hiding this comment.
CRITICAL #4 (shared) — Handler reads only tenant_id; caller_chittyid and operation from the schema are unused. Any authenticated caller can dump any tenant's full policy bundle (allowed_repos, allowed_remotes, default_author, protected_branches). No audit row is emitted for this endpoint either. Fix: enforce caller-tenant membership (note: no membership table exists today — fail closed with a clear reason code and follow-up issue), and emit event_type:"policy_resolve" audit (extend migration 019 CHECK constraint).
| "POLICY_BLOCKED_FORCE_TO_PROTECTED", | ||
| "Force-push to protected branch is hard-denied", | ||
| ); | ||
| } |
There was a problem hiding this comment.
CRITICAL #5 (code-reviewer) — /capabilities/confirm only checks protected_branches. It never reads policy.force_push_allowed, so a tenant where force-push is globally disabled at the scalar policy level can still successfully mint a confirmation token. Mint catches the second gate later, but confirm should fail-closed earlier with POLICY_BLOCKED_FORCE_DISABLED.
| remote TEXT, | ||
| ref TEXT, | ||
| force_class TEXT, | ||
| outcome TEXT NOT NULL CHECK(outcome IN ('ok','denied','expired','invalid')), |
There was a problem hiding this comment.
IMPORTANT #8 (silent-failure-hunter) — outcome CHECK allows expired and invalid but no code path writes them. Introspect on an expired record returns 200/active:false with no audit row. Confirm/ledger_emit JSON parse failures return 403 with no audit row. Forensic gap. Emit outcome:"expired" on the introspect-expired branch and outcome:"invalid" on parse-failure branches.
|
|
||
| await c.env.TOKEN_KV.put(CAP_PREFIX + token, JSON.stringify(record), { | ||
| expirationTtl: CAPABILITY_TTL_SECONDS, | ||
| }); |
There was a problem hiding this comment.
IMPORTANT #9 (silent-failure-hunter) — TOKEN_KV.put on mint and confirm is not wrapped in try/catch. If KV is degraded, the token returned to the caller is never persisted — caller treats it as live, introspect/confirm/emit all return 403. Wrap with try/catch; on failure return 503 POLICY_BLOCKED_CHITTYCONNECT_UNAVAILABLE and audit denied with reason TOKEN_STORE_UNAVAILABLE.
| // (durable D1 record). When the chittyledger domain handshake lands, the | ||
| // audit row will be forwarded to ChittyLedger and the ledger_event_id will | ||
| // be replaced with the upstream id. | ||
| // TODO(chittyledger#11): forward entry to ChittyLedger domain projection |
There was a problem hiding this comment.
DOC ROT #10 (comment-analyzer) — TODO references chittyledger#11, which is a merged Dependabot PR in that repo, not a tracking issue. Replace with the real tracking issue or remove the bracketed reference.
| /** | ||
| * Broker primitives v1 — capability mint/introspect/confirm, policy resolve, | ||
| * ledger emit. Closes chittyos/chittyconnect#210, #211. Partial of #209 | ||
| * (signing endpoints deferred to chittyos/chittyconnect#228). |
| "caller_chittyid": { | ||
| "type": "string", | ||
| "pattern": "^[A-Z0-9]{2}-[A-Z0-9]-[A-Z0-9]{3}-[A-Z0-9]{4}-[PLTEA]-[0-9]{4}-[0-5]-[0-9]{2}$", | ||
| "description": "Canonical ChittyID VV-G-LLL-SSSS-T-YYMM-C-XX per chittycanon://gov/governance." |
There was a problem hiding this comment.
DOC ROT #12 (comment-analyzer) — Description says VV-G-LLL-SSSS-T-YYMM-C-XX and the regex enforces [0-9]{4}-[0-5]-[0-9]{2} (consistent with the description). However the canonical ontology at chittycanon://gov/governance says VV-G-LLL-SSSS-T-YM-C-X (compact YM, single-digit C, single-digit X). Schema/description are internally consistent; canon disagrees with both. Recommend a follow-up reconciliation issue rather than silently changing either side — every existing fixture (CC-A-CCC-0001-P-2606-3-42) matches the schema, not canon.
…portant, 3 doc-rot Addresses inline review at #240. All real fixes — no mocks, no placeholders, no fake data. 19/19 broker tests pass (3 new), 461/461 suite passes. Critical: - #1 Hard-deny main/master is now a UNIVERSAL_PROTECTED_BRANCHES constant unioned into every tenant policy. Both bare (main/master) and prefixed (refs/heads/*) forms covered. Applied to mint AND confirm. - #2 Introspect now validates expires_at parses to a finite number. NaN (the previous fail-open) is treated as expired+invalid with audit. - #3 auditEvent split into best-effort (denied/expired/invalid) and critical (ok mint/confirm/ledger_emit). Critical throws → 503 AUDIT_WRITE_FAILED → KV rollback. Preserves audit-before-issue invariant. - #4 /policy/resolve now requires caller-tenant membership. No membership table exists in 016 yet (tenant_projects maps tenant→Neon only), so per the task constraint we fail-closed to canonical chittyos-default with a clear POLICY_BLOCKED_TENANT_MEMBERSHIP_UNVERIFIED reason. Tracking follow-up needed before widening. Emits policy_resolve audit rows. - #5 /capabilities/confirm now rejects when policy.force_push_allowed is false with POLICY_BLOCKED_FORCE_DISABLED, not just at mint time. - #6 Three new real-D1+KV tests added: expired-token introspect, four scope-mismatch redeem cases (#235 fix coverage), cross-tenant replay (#210 reopen reason). Important: - #7 git_remote_allowlist.match_type now dispatched (exact/prefix/glob); unknown match_type fails closed instead of falling through to glob. - #8 migration 019 CHECK constraint widened to allow expired/invalid/ policy_resolve outcomes; route handlers now emit them. - #9 TOKEN_KV.put on mint+confirm wrapped; KV failure returns 503 POLICY_BLOCKED_CHITTYCONNECT_UNAVAILABLE with TOKEN_STORE_UNAVAILABLE audit row. Doc rot: - #10 Removed incorrect chittyledger#11 reference (that was a merged Dependabot PR, not a tracking issue). - #11 src/index.js + broker-primitives.js header now reference #242 for signing-deferral; #228 is the credentials/secrets issue. - #12 ChittyID schema description left as-is (YYMM, matches regex); reported as canon-vs-schema mismatch needing reconciliation rather than silent edit — every existing fixture matches schema, not canon. Tracks: #209 #210 #211 #235 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b4f4ddf0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const scopeMatches = | ||
| crec.caller_chittyid === caller_chittyid && | ||
| crec.tenant_id === tenant_id && | ||
| crec.repo_path === repo_path && | ||
| crec.remote === remote && | ||
| crec.ref === ref && | ||
| crec.force_class === force_class; |
There was a problem hiding this comment.
When TOKEN_KV still returns a confirmation record after its embedded expires_at has passed (for example in local persistent KV, delayed/manual KV repair, or a stale restored record), this path only checks the bound scope and then mints a force capability. Because confirmation tokens are the freshness check for force pushes and are supposed to be valid for at most 120 seconds, redemption should compare crec.expires_at to Date.now() and fail closed before accepting the token.
Useful? React with 👍 / 👎.
| let policy; | ||
| try { | ||
| policy = await loadTenantPolicy(c.env, tenant_id); |
There was a problem hiding this comment.
For any authenticated API key, this loads policy for the tenant_id supplied in the request body and proceeds to mint if that tenant's repo/remote rows match; it never checks that the authenticated caller or claimed caller_chittyid belongs to that tenant. The new policy.resolve route explicitly fails closed because tenant membership cannot be verified, but the mint path skips the same gate, so an API-key holder who knows another configured tenant's repo prefix can obtain capability tokens under that tenant.
Useful? React with 👍 / 👎.
| addFormats(ajv); | ||
|
|
||
| export const validators = { | ||
| "capabilities.mint.input": ajv.compile(capabilitiesMintInput), |
There was a problem hiding this comment.
These module-scope ajv.compile(...) calls generate validator functions with dynamic code evaluation during Worker startup. Cloudflare Workers reject eval/new Function unless startup eval is explicitly enabled, and the inspected wrangler.jsonc compatibility flags only include nodejs_compat and global_fetch_strictly_public, so importing this route can make the deployed Worker fail to start; precompile the schemas or enable the required startup-eval path intentionally.
Useful? React with 👍 / 👎.
Summary
Implements 5 of 7 broker primitives specified in CHARTER.md "Git Broker Surface (REST, sensitive) — SPEC" (#235). Option A from prior audit: ships unblocked work now; signing endpoints deferred to #228.
/api/v1/capabilities/confirmsupersedes legacy/api/git/confirm/api/v1/policy/resolvebacked by migration 018 + new 019 extensionsWhy Option A
Signing requires 1Password key resolution + canonical commit/tag payload signing, both gated on #228. Rather than block all of #209, ship non-signing primitives now so
chittyagent-gitcan integrate against a real broker for capability mint/introspect/policy/audit. Signing lands in follow-up when #228 closes.Endpoints (5/7)
/api/v1/capabilities/mint/api/v1/capabilities/introspect/api/v1/capabilities/confirm/api/v1/policy/resolve/api/v1/ledger/emitLegacy
/api/git/confirm+/api/git/confirm/*return 410 Gone with replacement pointer.src/api/routes/git-confirm.jsdeleted.Storage (corrections from original brief, accepted from audit)
TOKEN_KV) for tokens, prefixedbroker:cap:/broker:confirm:— consistent with existing patternDB) for tenant policy + durable auditgit_tenant_policy(scalars per tenant)git_protected_branches(tuple list)broker_capability_audit(durable audit trail)ajv schemas validated: 10/10
src/schemas/v1/{capabilities.{mint,introspect,confirm},policy.resolve,ledger.emit}.{input,output}.json. ChittyID pattern enforces canonicalVV-G-LLL-SSSS-T-YYMM-C-XXperchittycanon://gov/governancewith all five {P,L,T,E,A} accepted.Tests — REAL D1 + KV
tests/api/v1/broker-primitives.test.jsbootswrangler unstable_devwith local D1 + KV. Migrations applied viawrangler d1 execute DB --env=dev --local --file=.... Novi.mockon DB/KV/service modules per global rule.Coverage: policy.resolve seed read-back; schema enforcement (positive+negative ChittyID, push requires ref+remote, force requires confirmation_token); repo allowlist denial; protected-branch hard-deny; force-push denied when
force_push_allowed=falseeven with valid confirmation token; mint→introspect roundtrip; ledger event_type↔operation binding; URL userinfo redaction; legacy 410.Test plan
tests/helpers/mocks.jspatterns NOT propagatedcurl -X POST https://chittyconnect-staging.chitty.cc/api/v1/policy/resolvereturns seeded policy/api/git/confirmreturns 410 with replacement pointer/api/v1/signing/*when chico-keys: provision credentials for git-mcp workstream (#209, #309, #111) #228 lands and closes [git-mcp] Implement git_commit / git_push / git_tag handlers #209🤖 Generated with Claude Code