Skip to content

feat: governance compliance cron, MCP tools, and govClient - #76

Merged
chitcommit merged 5 commits into
mainfrom
feat/governance-compliance-cron
Apr 6, 2026
Merged

chitcommit merged 5 commits into
mainfrom
feat/governance-compliance-cron

Conversation

@chitcommit

@chitcommit chitcommit commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

New scrape job types (sos_status, recorder_filings, assessor_check), govClient(), Cron Phase 11, MCP tools (query_compliance_calendar, verify_compliance_filing).

Summary by CodeRabbit

  • New Features

    • Integrated governance compliance data syncing with automatic daily synchronization
    • Added compliance calendar and deadline tracking capabilities
    • Enabled filing verification functionality
    • Extended job dispatch system to support Secretary of State status, recorder filings, and assessor check job types
  • Chores

    • Added environment configuration options for compliance service integration

…ation

Wires ChittyCommand as the orchestrator for governance/compliance monitoring:

- Job types: add sos_status, recorder_filings, assessor_check to
  ScrapeJobType union and all validation arrays (jobs, MCP, sync)
- govClient: new integration client for ChittyGov API (compliance
  calendar, verify filing, list monitors)
- Cron Phase 11: syncGovernanceCompliance() — pulls upcoming filings
  from ChittyGov (60-day window), upserts into cc_obligations as
  category='governance', enqueues verification scrapes for active monitors
- MCP tools: query_compliance_calendar and verify_compliance_filing
  proxying to ChittyGov
- Env: add CHITTYGOV_URL binding
- Sync routes: sos_status/recorder_filings/assessor_check as valid
  manual trigger sources

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Apr 6, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
chittycommand-ui 779b1af Apr 06 2026, 10:37 PM

@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81bc9b09-3f45-4287-95ef-6749cea51303

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6c80f and 779b1af.

📒 Files selected for processing (1)
  • src/index.ts

📝 Walkthrough

Walkthrough

This PR introduces a ChittyGov compliance integration by adding environment configuration bindings, a new integration layer with API client and data interfaces, a governance-focused cron phase that syncs compliance deadlines and enqueues verification jobs, new scrape job types, and MCP tools for querying and verifying filings.

Changes

Cohort / File(s) Summary
Environment & Integration Setup
src/index.ts, src/lib/integrations.ts
Added CHITTYGOV_URL and CHITTYGOV_TOKEN env bindings; introduced govClient factory function with ComplianceFiling and ComplianceMonitor interfaces; exposed three async methods: getComplianceCalendar, verifyFiling, and getMonitors, all with 10s timeout and error logging.
Cron Sync Implementation
src/lib/cron.ts, src/routes/sync.ts
Added Phase 11 cron phase for daily_api source that calls syncGovernanceCompliance; new exported function fetches compliance deadlines, upserts rows to cc_obligations with governance category, enqueues monitor scrape jobs, and returns synced count; wired new sync sources to dispatcher.
Job Type Expansion
src/lib/job-dispatcher.ts, src/routes/jobs.ts, src/routes/mcp.ts
Extended ScrapeJobType union with sos_status, recorder_filings, assessor_check; updated job validation and MCP tool schemas to accept and dispatch the three new job types.

Sequence Diagram

sequenceDiagram
    participant Cron as Cron Scheduler
    participant Sync as syncGovernanceCompliance
    participant Gov as ChittyGov API
    participant DB as Database
    participant Queue as Job Queue
    
    Cron->>Sync: trigger with env, sql
    Sync->>Gov: getComplianceCalendar(60 days)
    Gov-->>Sync: {filings, total}
    Sync->>DB: upsert filing → cc_obligations
    Sync->>DB: update existing obligation metadata
    Sync->>DB: fetch active monitors with scraperId
    Sync->>Queue: enqueueJob(scrape payload)
    Queue-->>Sync: job enqueued
    Sync-->>Cron: return synced count
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 A compliance leap, we've made it so,
With ChittyGov's winds that blow and flow,
Deadlines sync'd and jobs enqueued,
Governance filings now reviewed! 🎯📋

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main changes: it introduces governance compliance cron functionality, MCP tools, and the govClient integration—all primary objectives reflected in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/governance-compliance-cron

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 363a2a5e88

ℹ️ 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".

Comment thread src/lib/cron.ts Outdated
Comment thread src/routes/sync.ts
…ed counter

Addresses review findings on PR #76:
- Add CHITTYGOV_TOKEN env + Bearer auth header (was missing entirely)
- Log HTTP status + response body on non-ok responses (was returning null silently)
- Track failed filing count and log summary on partial failures
- Fix inverted latePenalty logic (was writing 0 instead of actual penalty)
- Add Array.isArray guard on filings response + NaN guard on fee/penalty
- Warn explicitly when getMonitors returns null instead of silent skip
- Isolate KV read failure with own try-catch and clear label

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chitcommit

Copy link
Copy Markdown
Contributor Author

Review remediation pushed (e4e3f71)

Fixed (7 issues):

  • HIGH: Added CHITTYGOV_TOKEN env + Bearer auth header — was completely missing
  • HIGH: All 3 govClient methods now log HTTP status + response body on failure — was returning null silently
  • HIGH: Failed filing counter + summary log — partial sync failures were invisible
  • MEDIUM: latePenalty logic was inverted (wrote 0 instead of actual penalty) — fixed
  • MEDIUM: Added Array.isArray guard on filings + isNaN guard on fee/penalty
  • MEDIUM: Explicit warn when getMonitors returns null (was silent)
  • LOW: KV read isolated with own try-catch and clear error label

Design notes (not blocking, for future):

  1. MCP tool failures return as successful responses (no isError flag) — pre-existing pattern across all tools, not specific to this PR
  2. Three sync sources (sos_status, recorder_filings, assessor_check) all route to syncGovernanceCompliance — consider passing source name for scoped sync, or consolidating to a single governance source

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/lib/job-dispatcher.ts (1)

6-13: Export the scrape job types as a shared runtime constant.

This 7-value set is now duplicated in src/routes/jobs.ts Line 68 and src/routes/mcp.ts Lines 305, 351, and 1190. A SCRAPE_JOB_TYPES tuple here would keep compile-time and runtime validation from drifting on the next addition.

Possible refactor
-export type ScrapeJobType =
-  | 'court_docket'
-  | 'cook_county_tax'
-  | 'mr_cooper'
-  | 'portal_scrape'
-  | 'sos_status'
-  | 'recorder_filings'
-  | 'assessor_check';
+export const SCRAPE_JOB_TYPES = [
+  'court_docket',
+  'cook_county_tax',
+  'mr_cooper',
+  'portal_scrape',
+  'sos_status',
+  'recorder_filings',
+  'assessor_check',
+] as const;
+
+export type ScrapeJobType = typeof SCRAPE_JOB_TYPES[number];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/job-dispatcher.ts` around lines 6 - 13, Add and export a runtime
tuple constant named SCRAPE_JOB_TYPES containing the seven string literals
('court_docket','cook_county_tax','mr_cooper','portal_scrape','sos_status','recorder_filings','assessor_check')
and change the ScrapeJobType type to be derived from it (type ScrapeJobType =
typeof SCRAPE_JOB_TYPES[number]); export both so other modules can import
SCRAPE_JOB_TYPES instead of duplicating the list and use ScrapeJobType for
typing, and update callers in routes to import the constant (and remove their
local duplicates).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/index.ts`:
- Around line 56-57: The Env type was extended with CHITTYGOV_URL and
CHITTYGOV_TOKEN but wrangler.toml never provides CHITTYGOV_URL and the token
must be a secret, causing govClient() to return null and MCP routes in
src/routes/mcp.ts to no-op; fix by adding CHITTYGOV_URL to the wrangler.toml
[vars] section (e.g., CHITTYGOV_URL = "https://gov.chitty.cc") and provisioning
CHITTYGOV_TOKEN via the CLI (wrangler secret put CHITTYGOV_TOKEN) so
src/lib/integrations.ts can read the secret at runtime.

In `@src/lib/cron.ts`:
- Around line 913-923: The UPDATE SQL for cc_obligations is missing refresh of
the canonical late_fee column, so modify the UPDATE inside the sql`` block that
sets status/due_date/amount_due/metadata/updated_at to also set late_fee =
${filing.latePenalty} (or CASE WHEN appropriate) so the stored late_fee is
updated when a filing already exists; likewise ensure the INSERT path (the
UPSERT/INSERT block around lines 928-937) includes late_fee =
${filing.latePenalty} in its column list/VALUES so new rows get the same value.
Use the existing symbols (cc_obligations, filing.latePenalty, amount,
jsonb_set/metadata, updated_at) to locate and update both the update and insert
SQL fragments.
- Around line 905-910: Replace the current read-then-insert flow against the
cc_obligations table (the SELECT that looks up metadata->>'filing_id' and the
subsequent INSERT block that runs when no row is found) with a single DB-backed
upsert: create an expression unique index on (category,
(metadata->>'filing_id')) to enforce uniqueness, then perform INSERT ... ON
CONFLICT (...) DO UPDATE to atomically create-or-update the governance
obligation for the given filing.filingId instead of using the existing variable
read path; update the code in src/lib/cron.ts (the code that assigns const
[existing] from cc_obligations and the insert block that follows) to use this
upsert and remove the race-prone SELECT branch.

In `@src/routes/mcp.ts`:
- Around line 373-381: The inputSchema for the 'verify_compliance_filing' route
declares source values as 'scrape', 'email', or 'manual' but elsewhere the
omitted-inputs default is set to 'mcp', producing an undocumented provenance;
update the omitted/default handling to use one of the documented values (e.g.,
'manual') instead of 'mcp'. Locate the route named verify_compliance_filing and
the omittedInputs/default assignment (the other occurrence around the
omittedInputs code block referenced near lines ~1213-1223) and change the
default/source fallback to a documented value ('scrape' | 'email' | 'manual') so
the emitted provenance matches the inputSchema and upstream validation.

---

Nitpick comments:
In `@src/lib/job-dispatcher.ts`:
- Around line 6-13: Add and export a runtime tuple constant named
SCRAPE_JOB_TYPES containing the seven string literals
('court_docket','cook_county_tax','mr_cooper','portal_scrape','sos_status','recorder_filings','assessor_check')
and change the ScrapeJobType type to be derived from it (type ScrapeJobType =
typeof SCRAPE_JOB_TYPES[number]); export both so other modules can import
SCRAPE_JOB_TYPES instead of duplicating the list and use ScrapeJobType for
typing, and update callers in routes to import the constant (and remove their
local duplicates).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 55c8ac14-7ea8-4ae6-8047-cc87563038fa

📥 Commits

Reviewing files that changed from the base of the PR and between fbd802c and e4e3f71.

📒 Files selected for processing (7)
  • src/index.ts
  • src/lib/cron.ts
  • src/lib/integrations.ts
  • src/lib/job-dispatcher.ts
  • src/routes/jobs.ts
  • src/routes/mcp.ts
  • src/routes/sync.ts

Comment thread src/index.ts
Comment thread src/lib/cron.ts
Comment thread src/lib/cron.ts
Comment thread src/routes/mcp.ts
Sets gov.chitty.cc service URL in [vars] alongside other service URLs.
Token (CHITTYGOV_TOKEN) will be provisioned via ChittyConnect when
ChittyGov adds auth — govClient already handles it conditionally.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read all changed files for adversarial review
echo "=== src/lib/integrations.ts (govClient section) ==="
# Find and show the govClient implementation
rg -n "govClient|CHITTYGOV|ComplianceFiling|ComplianceMonitor|AbortSignal|Bearer|X-Source" src/lib/integrations.ts -A 3 -B 1

echo ""
echo "=== src/lib/cron.ts (syncGovernanceCompliance) ==="
grep -n "syncGovernanceCompliance\|cc_obligations\|enqueueJob\|govClient\|governance\|failed\|synced\|latePenalty\|isNaN\|Array.isArray\|KV\|kv\|CHITTYGOV" src/lib/cron.ts -A 2 -B 1

echo ""
echo "=== src/routes/mcp.ts (new tools) ==="
grep -n "query_compliance_calendar\|verify_compliance_filing\|govClient\|isError\|sos_status\|recorder_filings\|assessor_check\|validTypes\|enqueue_scrape_job\|query_scrape_jobs" src/routes/mcp.ts -A 5 -B 2

echo ""
echo "=== src/routes/jobs.ts ==="
grep -n "validTypes\|sos_status\|recorder_filings\|assessor_check\|job_type\|auth\|Auth\|token\|Token" src/routes/jobs.ts -A 3 -B 2

echo ""
echo "=== src/routes/sync.ts ==="
grep -n "sos_status\|recorder_filings\|assessor_check\|syncGovernanceCompliance\|trigger\|auth\|token\|validSources\|source" src/routes/sync.ts -A 3 -B 2

echo ""
echo "=== src/lib/job-dispatcher.ts ==="
grep -n "ScrapeJobType\|sos_status\|recorder_filings\|assessor_check\|validat" src/lib/job-dispatcher.ts -A 3 -B 2

echo ""
echo "=== src/index.ts (Env) ==="
grep -n "CHITTYGOV\|Env\|env" src/index.ts -A 2 -B 1

Length of output: 37116


🏁 Script executed:

#!/bin/bash
echo "=== verifyFiling implementation in integrations.ts ==="
grep -n "verifyFiling\|verify\|filing_id\|filingId\|encodeURI" src/lib/integrations.ts -A 5 -B 2

echo ""
echo "=== Monitor loop + typeMap in cron.ts ==="
# Get the full syncGovernanceCompliance function
awk '/export async function syncGovernanceCompliance/,/^}/' src/lib/cron.ts

echo ""
echo "=== MCP auth middleware check ==="
grep -n "auth\|Auth\|bearer\|Bearer\|token\|Token\|middleware\|use(" src/routes/mcp.ts -A 2 -B 1 | head -60

echo ""
echo "=== sync.ts auth check ==="
grep -n "auth\|Auth\|bearer\|Bearer\|token\|Token\|middleware\|use(" src/routes/sync.ts -A 2 -B 1

echo ""
echo "=== index.ts route mounting + auth middleware ==="
grep -n "use\|app\.\|auth\|sync\|mcp\|jobs" src/index.ts -A 1 -B 1 | head -80

echo ""
echo "=== CHITTYGOV_TOKEN warning if missing ==="
grep -n "CHITTYGOV_TOKEN\|warn.*token\|token.*warn" src/lib/integrations.ts -A 3 -B 2

Length of output: 15306


@chitcommit — adversarial review complete. Here's the full security, correctness, and regression risk analysis against the hardened commit (e4e3f71):


Adversarial Review — PR #76

🔴 HIGH — scrapeInput spread overrides hardcoded job target fields

File: src/lib/cron.tssyncGovernanceCompliance

await enqueueJob(sql, jobType, {
  scraper_id: monitor.scraperId,
  monitor_id: monitor.monitorId,
  ...((monitor.scrapeInput as Record<string, unknown>) || {}),  // ← runs AFTER, overrides above
}, { chittyId, cronSource: 'governance' }, env);

monitor.scrapeInput is spread after scraper_id and monitor_id, so a ChittyGov response containing scrapeInput: { scraper_id: "attacker-controlled" } silently overrides the intended values. chittyId and cronSource are in a separate opts argument and are safe — but the target payload itself is vulnerable to a compromised or misconfigured ChittyGov response redirecting scrape jobs.

Fix: Move the spread first:

await enqueueJob(sql, jobType, {
  ...((monitor.scrapeInput as Record<string, unknown>) || {}),
  scraper_id: monitor.scraperId,   // explicit fields win
  monitor_id: monitor.monitorId,
}, { chittyId, cronSource: 'governance' }, env);

🔴 HIGH — Silent unauthenticated verifyFiling when CHITTYGOV_TOKEN unset

File: src/lib/integrations.tsgovClient

if (env.CHITTYGOV_TOKEN) {
  headers['Authorization'] = `Bearer ${env.CHITTYGOV_TOKEN}`;
}
// No warn/error if token is absent

Unlike the scrape:service_token pattern (which emits console.warn('[router] No scrape:service_token in KV — requests will be unauthenticated')), govClient silently proceeds without auth. verifyFiling is a state-mutating POST that triggers recurrence roll-forward in ChittyGov. If CHITTYGOV_TOKEN is accidentally unset in a new deployment, this mutation fires unauthenticated. Add a warn at construction time:

if (!env.CHITTYGOV_TOKEN) {
  console.warn('[gov] CHITTYGOV_TOKEN not set — requests will be unauthenticated');
}

🟡 MEDIUM — Triple-enqueue on concurrent manual trigger

File: src/routes/sync.ts

sos_status:        () => syncGovernanceCompliance(c.env, sql),
recorder_filings:  () => syncGovernanceCompliance(c.env, sql),
assessor_check:    () => syncGovernanceCompliance(c.env, sql),

All three sources call the full syncGovernanceCompliance, which runs getMonitors('active') and enqueues all monitor jobs on every invocation. If any two of these are triggered concurrently (or in quick succession, e.g. from a CI/automation loop or a retry), the same monitor verification scrapes are enqueued 2–3×. The DB upsert for cc_obligations is idempotent (keyed on filing_id), but enqueueJob inserts a new queue row each time — no dedup check is visible in the dispatcher. This can cause duplicated scrape execution and inflated cc_sync_log counts.

Suggested mitigations (pick one):

  • Consolidate to a single governance trigger source.
  • Or add a uniqueness constraint / "queued check" in enqueueJob for governance-sourced jobs.
  • Or scope each source to its monitor type subset (aligns with @chitcommit's Design Note #2).

🟡 MEDIUM — late_fee not updated on existing obligation UPDATE

File: src/lib/cron.tssyncGovernanceCompliance UPDATE branch

UPDATE cc_obligations
SET status    = ...,
    due_date  = ...,
    amount_due = CASE WHEN ${amount} > 0 THEN ${amount} ELSE amount_due END,
    metadata  = jsonb_set(...),
    updated_at = NOW()
WHERE id = ${existing.id}

late_fee is populated on INSERT but never updated on subsequent syncs. If ChittyGov revises a penalty (e.g. statutory change), the local row retains the stale value indefinitely. This is not a security issue but creates a silent correctness gap for financial data. Add late_fee = COALESCE(NULLIF(${latePenalty}::numeric, 0), late_fee) or an unconditional update with the same null-guard pattern as amount_due.


🟡 MEDIUM — getMonitors URL injection (fragile, currently safe)

File: src/lib/integrations.tsgetMonitors

const qs = status ? `?status=${status}` : '';
const res = await fetch(`${govUrl}/api/compliance/monitors${qs}`, ...);

status is raw-concatenated into the URL without encodeURIComponent or URLSearchParams. Currently the only caller passes the hardcoded string 'active', so no immediate exploit path. But unlike every other method in govClient (which uses URLSearchParams), this is an inconsistency. If a future caller passes a dynamic value (e.g. from MCP args), &inject=true becomes a valid query param injection. Use URLSearchParams for consistency:

const qs = status ? `?${new URLSearchParams({ status })}` : '';

🟢 LOW — authority_url stored unvalidated in metadata

File: src/lib/cron.ts — INSERT path

authority_url: filing.authorityUrl,  // raw from ChittyGov API

No scheme or format check. A compromised ChittyGov response could write javascript: or data: URLs into cc_obligations.metadata. If any UI renders this as a hyperlink without sanitization, it becomes a stored XSS vector. Low risk given it's a JSON metadata field, but worth a https?:// guard at ingest.


✅ Confirmed Clean

Concern Result
verifyFiling path traversal via filing_id Safe — encodeURIComponent used ✅
SQL injection via filing.filingId Safe — parameterized Neon queries ✅
syncGovernanceCompliance auth bypass Safe — /api/sync is under app.use('/api/*', authMiddleware)
getMonitors enqueuing inactive monitors Safe — called with 'active' filter ✅
Unknown monitorType in typeMap Safe — if (!jobType) continue guard ✅
MCP enqueue_scrape_job job_type validation Safe — explicit allowlist ✅
Array.isArray guard on filings Present ✅
isNaN guard on fee/penalty Present ✅
Failed filing counter visible in logs Present ✅

Merge gate assessment: The scrapeInput spread order (🔴) is the highest-priority fix before merge — it's a supply-chain confused-deputy issue exploitable via a compromised ChittyGov response. The silent unauthenticated verifyFiling (🔴) is a deployment-config footgun worth one line of warn. The triple-enqueue (🟡) is a real operational bug but not blocking if ChittyGov API rate limits are generous. The remaining items are correctness/hygiene.


🧠 Learnings used
Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-04T04:42:57.380Z
Learning: Resolve must-fix review comments (security, correctness, compliance, merge blockers) before merge

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T18:02:56.403Z
Learning: Applies to src/lib/cron.ts : Sync Notion tasks to cc_tasks table via Phase 9 cron and reconcile legal tasks to cc_disputes via Phase 10 cron

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T16:02:20.597Z
Learning: Applies to src/lib/cron.ts : Orchestrate all data source syncs (Plaid, ChittyFinance, court docket, utility scrapers, mortgage, property tax) in `src/lib/cron.ts` as the cron sync orchestrator
✅ Actions performed

Review triggered.

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.

@chitcommit
chitcommit enabled auto-merge (squash) April 6, 2026 22:32
@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== scrapeInput spread order check (cron.ts) ==="
grep -n "scrapeInput\|scraper_id\|monitor_id\|enqueueJob" src/lib/cron.ts -A 5 -B 2

echo ""
echo "=== CHITTYGOV_TOKEN warn check (integrations.ts) ==="
grep -n "CHITTYGOV_TOKEN\|warn\|unauthenticated\|Bearer\|Authorization" src/lib/integrations.ts -A 3 -B 2

echo ""
echo "=== getMonitors URLSearchParams check ==="
grep -n "getMonitors\|URLSearchParams\|status\|encodeURI\|qs\b" src/lib/integrations.ts -A 5 -B 2

echo ""
echo "=== late_fee update check on UPDATE path (cron.ts) ==="
grep -n "late_fee\|latePenalty\|UPDATE\|amount_due\|SET\b" src/lib/cron.ts -A 8 -B 2

echo ""
echo "=== authority_url / authorityUrl validation check ==="
grep -n "authorityUrl\|authority_url\|https\|validate\|scheme\|sanitiz" src/lib/cron.ts -A 3 -B 2

echo ""
echo "=== Full syncGovernanceCompliance function ==="
awk '/export async function syncGovernanceCompliance/,/^}/' src/lib/cron.ts

echo ""
echo "=== Full govClient function ==="
awk '/export function govClient/,/^}/' src/lib/integrations.ts

Length of output: 45921


@github-actions — adversarial re-review complete against the current branch HEAD. All issues from the prior pass were verified against live file state. None of the 6 previously flagged issues have been addressed. Status below:


Adversarial Re-Review — PR #76 (Current HEAD)

🔴 HIGH — scrapeInput spread still overrides explicit target fields (cron.ts Lines 985–988)

await enqueueJob(sql, jobType, {
  scraper_id: monitor.scraperId,
  monitor_id: monitor.monitorId,
  ...((monitor.scrapeInput as Record<string, unknown>) || {}), // ← STILL runs last
}, ...);

Unchanged. A compromised ChittyGov response with scrapeInput: { scraper_id: "attacker-value" } silently redirects scrape jobs. Fix: move the spread first.


🔴 HIGH — Silent unauthenticated requests when CHITTYGOV_TOKEN unset (integrations.ts Lines 1036–1038)

if (env.CHITTYGOV_TOKEN) {
  headers['Authorization'] = `Bearer ${env.CHITTYGOV_TOKEN}`;
}
// ← No warn. verifyFiling (state-mutating POST) fires unauthenticated silently.

Unchanged. Contrast with [router] No scrape:service_token in KV — requests will be unauthenticated pattern used elsewhere in the same file. One line of console.warn at govClient construction time resolves this.


🟡 MEDIUM — Triple-enqueue remains on concurrent trigger (sync.ts)

All three sources (sos_status, recorder_filings, assessor_check) still call the full syncGovernanceCompliance, which runs getMonitors('active') and enqueues all active monitor jobs on every invocation. Concurrent or rapid-succession triggers cause duplicate enqueueJob inserts. Unchanged.


🟡 MEDIUM — late_fee never updated on obligation UPDATE (cron.ts Lines 913–925)

The UPDATE branch sets status, due_date, amount_due (conditionally), and metadata.last_gov_sync, but late_fee is omitted. If ChittyGov revises a penalty, the DB row retains the stale value forever. The latePenalty variable is computed but only consumed in the INSERT branch. Unchanged.

Suggested addition to the UPDATE SET clause:

late_fee = COALESCE(NULLIF(${latePenalty}::numeric, 0), late_fee),

🟡 MEDIUM — getMonitors raw string concatenation vs. URLSearchParams (integrations.ts Line 1082)

const qs = status ? `?status=${status}` : '';  // ← raw concat, no encoding

Every other parameterized URL in govClient and across integrations.ts uses URLSearchParams. The current caller passes the hardcoded 'active' literal so no active exploit path — but this is an inconsistency that becomes injectable if a dynamic value (e.g. from an MCP arg) is ever passed. Unchanged.


🟢 LOW — authority_url written unvalidated to metadata (cron.ts Line 942)

authority_url: filing.authorityUrl,  // raw from ChittyGov — no scheme check

A javascript: or data: URL survives into cc_obligations.metadata. No scheme guard added. Unchanged.


Merge Gate Summary

# Severity Issue Status
1 🔴 HIGH scrapeInput spread overrides scraper_id/monitor_id Open
2 🔴 HIGH Silent unauthenticated verifyFiling when token unset Open
3 🟡 MEDIUM Triple-enqueue on concurrent trigger Open
4 🟡 MEDIUM late_fee not updated on obligation UPDATE Open
5 🟡 MEDIUM getMonitors URL raw-concat vs. URLSearchParams Open
6 🟢 LOW authority_url unvalidated in metadata Open

Per repo policy, the two 🔴 HIGH items are merge-blocking. Items 3–5 are functional correctness issues that should be resolved before merge given this is financial/compliance data. Item 6 is risk-conditional on downstream rendering.


🧠 Learnings used
Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-04T04:42:57.380Z
Learning: Resolve must-fix review comments (security, correctness, compliance, merge blockers) before merge

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T18:02:56.403Z
Learning: Applies to src/lib/cron.ts : Sync Notion tasks to cc_tasks table via Phase 9 cron and reconcile legal tasks to cc_disputes via Phase 10 cron

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T16:02:20.597Z
Learning: Applies to src/lib/cron.ts : Orchestrate all data source syncs (Plaid, ChittyFinance, court docket, utility scrapers, mortgage, property tax) in `src/lib/cron.ts` as the cron sync orchestrator

@github-actions

github-actions Bot commented Apr 6, 2026

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chitcommit
chitcommit merged commit 52c24c4 into main Apr 6, 2026
10 of 14 checks passed
@chitcommit
chitcommit deleted the feat/governance-compliance-cron branch April 6, 2026 22:37
@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Git log: recent commits ==="
git log --oneline -10

echo ""
echo "=== HIGH `#1`: scrapeInput spread order (cron.ts) ==="
grep -n "scrapeInput\|scraper_id\|monitor_id\|enqueueJob" src/lib/cron.ts -A 5 -B 2

echo ""
echo "=== HIGH `#2`: CHITTYGOV_TOKEN silent unauth warn (integrations.ts) ==="
grep -n "CHITTYGOV_TOKEN\|warn\|unauthenticated\|Bearer\|Authorization\|govClient" src/lib/integrations.ts -A 3 -B 2

echo ""
echo "=== MEDIUM `#3`: triple-enqueue (sync.ts) ==="
grep -n "sos_status\|recorder_filings\|assessor_check\|syncGovernanceCompliance\|governance" src/routes/sync.ts -A 3 -B 2

echo ""
echo "=== MEDIUM `#4`: late_fee UPDATE branch (cron.ts) ==="
grep -n "late_fee\|latePenalty\|SET\b\|UPDATE\|amount_due" src/lib/cron.ts -A 8 -B 2

echo ""
echo "=== MEDIUM `#5`: getMonitors URLSearchParams (integrations.ts) ==="
grep -n "getMonitors\|URLSearchParams\|encodeURI\|\bqs\b\|status\b" src/lib/integrations.ts -A 5 -B 2

echo ""
echo "=== LOW `#6`: authority_url unvalidated (cron.ts) ==="
grep -n "authorityUrl\|authority_url\|https\b\|validate\|scheme\|sanitiz\|startsWith" src/lib/cron.ts -A 3 -B 2

echo ""
echo "=== MCP: isError flag / merge-gate bypass vectors (mcp.ts) ==="
grep -n "isError\|query_compliance_calendar\|verify_compliance_filing\|govClient\|error\|Error\|throw\|status\|200\|400\|401\|403" src/routes/mcp.ts -A 4 -B 2

echo ""
echo "=== Policy bypass: /api/sync auth middleware check ==="
grep -n "authMiddleware\|use\('/api\|use(\"/api\|app\.use\|router\.use\|middleware" src/index.ts -A 2 -B 2

echo ""
echo "=== jobs.ts: auth + validTypes check ==="
grep -n "validTypes\|auth\|token\|401\|403\|job_type\|allowlist" src/routes/jobs.ts -A 3 -B 2

echo ""
echo "=== New injection vectors: filingId encodeURIComponent (integrations.ts) ==="
grep -n "encodeURI\|filingId\|filing_id\|verify\b\|POST\b\|GET\b\|fetch(" src/lib/integrations.ts -A 4 -B 2

echo ""
echo "=== enqueueJob implementation — dedup/idempotency check (job-dispatcher.ts) ==="
cat src/lib/job-dispatcher.ts

echo ""
echo "=== Full syncGovernanceCompliance function ==="
awk '/export async function syncGovernanceCompliance/,/^}/' src/lib/cron.ts

echo ""
echo "=== Full govClient function ==="
awk '/export function govClient/,/^}/' src/lib/integrations.ts

echo ""
echo "=== Regression check: runCronSync daily_api phase ordering ==="
grep -n "daily_api\|Phase\|syncGovernance\|syncPlaid\|syncFinance\|syncCourt\|syncMortgage\|syncProperty\|recordsSynced" src/lib/cron.ts -A 5 -B 2

Length of output: 128523


@github-actions — adversarial re-review complete against current HEAD (post 779b1af merge). One new HIGH deployment regression discovered. All 6 prior issues remain unaddressed.


Adversarial Review — PR #76 (Pass 3, post main-merge)

🔴 NEW HIGH — CHITTYGOV_URL silently absent after main merge

Git log:

779b1af Merge branch 'main' into feat/governance-compliance-cron
774a82c Revert "chore: add CHITTYGOV_URL to wrangler vars"   ← from main
...
9d6c80f chore: add CHITTYGOV_URL to wrangler vars            ← this PR

The revert of CHITTYGOV_URL from main (774a82c) was merged into this branch. On the current HEAD, wrangler.toml does not contain CHITTYGOV_URL. At deploy time, env.CHITTYGOV_URL will be undefined, govClient(env) returns null, and Phase 11 (syncGovernanceCompliance) logs [governance] ChittyGov not configured — skipping and returns 0 — with no deployment error and no visible signal to an operator. The entire Phase 11, all three manual trigger routes, and both MCP governance tools are silently dead.

Fix: Re-add CHITTYGOV_URL = "https://gov.chitty.cc" to wrangler.toml after reconciling with main.


🔴 HIGH #1scrapeInput spread still overrides explicit target fields (cron.ts Lines 985–988)

await enqueueJob(sql, jobType, {
  scraper_id: monitor.scraperId,
  monitor_id: monitor.monitorId,
  ...((monitor.scrapeInput as Record<string, unknown>) || {}), // ← STILL last
}, ...);

Unchanged. A compromised ChittyGov scrapeInput: { scraper_id: "attacker" } silently redirects scrape jobs. Move the spread first.


🔴 HIGH #2 — Silent unauthenticated requests when CHITTYGOV_TOKEN unset (integrations.ts Lines 1036–1038)

if (env.CHITTYGOV_TOKEN) {
  headers['Authorization'] = `Bearer ${env.CHITTYGOV_TOKEN}`;
}
// ← No warn. verifyFiling (state-mutating POST) fires unauthenticated silently.

Unchanged. The [router] No scrape:service_token in KV — requests will be unauthenticated pattern used elsewhere is absent here.


🟡 MEDIUM #3 — Triple-enqueue confirmed with no dedup (sync.ts + job-dispatcher.ts)

sos_status:       () => syncGovernanceCompliance(c.env, sql),
recorder_filings: () => syncGovernanceCompliance(c.env, sql),
assessor_check:   () => syncGovernanceCompliance(c.env, sql),

Unchanged, and now confirmed concrete: enqueueJob's local Neon fallback is a bare INSERT INTO cc_scrape_jobs with no ON CONFLICT clause. The ScrapeAgent path proxies through to ChittyRouter with no dedup check. Concurrent triggers produce N×duplicate job rows — no idempotency gate anywhere in the stack.


🟡 MEDIUM #4late_fee never updated on obligation UPDATE (cron.ts Lines 913–925)

latePenalty is computed correctly (post prior fix) but consumed only in the INSERT branch. The UPDATE SET clause still omits it. Unchanged.

-- Missing from UPDATE:
late_fee = COALESCE(NULLIF(${latePenalty}::numeric, 0), late_fee),

🟡 MEDIUM #5getMonitors raw URL concatenation (integrations.ts Line 1082)

const qs = status ? `?status=${status}` : '';  // ← raw concat

Every other parameterized URL in govClient and across integrations.ts uses URLSearchParams. Unchanged.


🟢 LOW #6authority_url written unvalidated to metadata (cron.ts Line 942)

authority_url: filing.authorityUrl,  // raw from ChittyGov — no scheme check

No https?:// guard. Unchanged.


🔵 INFORMATIONAL — MCP trigger_sync governance gap

mcp.ts Line 1045 validSources does not include sos_status, recorder_filings, or assessor_check. Governance syncs cannot be triggered via MCP's trigger_sync tool — only via REST /api/sync/trigger/:source. Non-blocking but creates an inconsistency between MCP and REST surface areas.


Updated Merge Gate

# Severity Issue Status
0 🔴 NEW HIGH CHITTYGOV_URL absent from wrangler.toml after main merge — Phase 11 silently no-ops on deploy Open
1 🔴 HIGH scrapeInput spread overrides scraper_id/monitor_id (supply-chain confused-deputy) Open
2 🔴 HIGH Silent unauthenticated verifyFiling when CHITTYGOV_TOKEN unset Open
3 🟡 MEDIUM Triple-enqueue on concurrent trigger — dedup absence now confirmed in dispatcher code Open
4 🟡 MEDIUM late_fee not updated on obligation UPDATE Open
5 🟡 MEDIUM getMonitors raw URL concat vs. URLSearchParams Open
6 🟢 LOW authority_url unvalidated in metadata Open

Three 🔴 HIGH items are merge-blocking per repo policy. Items 3–5 affect financial/compliance data correctness. The new item #0 means the feature is functionally inert post-deploy until resolved.


🧠 Learnings used
Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T16:02:20.597Z
Learning: Applies to src/lib/cron.ts : Orchestrate all data source syncs (Plaid, ChittyFinance, court docket, utility scrapers, mortgage, property tax) in `src/lib/cron.ts` as the cron sync orchestrator

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T18:02:56.403Z
Learning: Applies to src/lib/cron.ts : Sync Notion tasks to cc_tasks table via Phase 9 cron and reconcile legal tasks to cc_disputes via Phase 10 cron

Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-04T04:42:57.380Z
Learning: Resolve must-fix review comments (security, correctness, compliance, merge blockers) before merge

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Review remediation pushed (e4e3f71)

Fixed (7 issues):...

The remediation in e4e3f71 is solid — all 7 issues look correct.

While investigating, I also found two CI failures on this PR that the remediation didn't cover: npm audit --audit-level=high was failing due to:

  • lodash ≤4.17.23 (HIGH — Code Injection + Prototype Pollution via json-schema-to-typescriptagents transitive chain)
  • vite 7.0.0–7.3.1 (HIGH — Path Traversal + Arbitrary File Read via vitest transitive dep)

I prepared the fix (commit caf9898 locally):

  • Added lodash: "^4.18.0" and vite: "^7.3.2" to overrides in root package.json
  • Updated ui/package.json vite pin from ^7.3.1^7.3.2
  • Regenerated both lock files — npm audit --audit-level=high exits 0
  • Updated MCP tool count test from 4850 (the 2 new governance tools added in this PR)

The branch was deleted after merge so I can't push to it — these changes need a follow-up commit on main (or a new patch PR). CodeQL and code review are both clean on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants