Skip to content

fix(orb): authenticate and harden /v1/orb/ingest - #1248

Closed
JSONbored wants to merge 2 commits into
mainfrom
codex/propose-fix-for-orb-ingest-vulnerability
Closed

fix(orb): authenticate and harden /v1/orb/ingest#1248
JSONbored wants to merge 2 commits into
mainfrom
codex/propose-fix-for-orb-ingest-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The public /v1/orb/ingest receiver accepted unauthenticated batches and parsed the full request body before any limits or signature checks, allowing remote attackers to poison the central orb_signals dataset and cause write/storage amplification.

Description

  • Require a valid x-orb-signature HMAC (verified via ORB_INGEST_SECRET) before parsing or inserting any ingest payload by adding verifyOrbIngestSignature and checking it in the route handler.
  • Limit transport size with a streaming reader readOrbIngestBody and a 128 KiB cap that returns 413 on overflow so oversized JSON cannot be fully read into memory.
  • Harden payload validation and database inputs by bounding instance_id, repo_hash, pr_hash, gate_verdict, and created_at lengths and normalizing optional fields before INSERT.
  • Update OpenAPI to document the x-orb-signature header and new 401/413 responses and regenerate the UI OpenAPI artifact, and add integration tests covering signed success, unsigned/oversized negative paths, and bounded-field validation.
  • Expose ORB_INGEST_SECRET in Env type (src/env.d.ts) for runtime configuration.

Testing

  • Ran npx vitest run test/integration/orb-ingest.test.ts and all integration tests passed (34/34).
  • Regenerated and checked the UI OpenAPI with npm run ui:openapi and npm run ui:openapi:check, which succeeded and updated apps/gittensory-ui/public/openapi.json.
  • Some repository-wide checks were blocked locally: npm run typecheck failed due to missing optional self-host package type declarations (pg, ioredis), npm run test:coverage failed in the local toolchain with a coverage remap error (TypeError: jsTokens is not a function), and npm audit --audit-level=moderate returned 403 from the registry; these are environmental/tooling issues and did not affect the added integration tests.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 24, 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 Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 8427c2d Commit Preview URL

Branch Preview URL
Jun 24 2026, 06:31 PM

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.97%. Comparing base (ff0eade) to head (8517fa3).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/orb/ingest.ts 94.59% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1248      +/-   ##
==========================================
- Coverage   94.97%   94.97%   -0.01%     
==========================================
  Files         177      177              
  Lines       19806    19843      +37     
  Branches     7125     7139      +14     
==========================================
+ Hits        18811    18846      +35     
  Misses        397      397              
- Partials      598      600       +2     
Files with missing lines Coverage Δ
src/api/routes.ts 94.40% <100.00%> (+0.01%) ⬆️
src/openapi/spec.ts 99.39% <ø> (ø)
src/orb/ingest.ts 96.49% <94.59%> (-3.51%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jun 24, 2026

@superagent-security superagent-security 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.

Superagent found 1 security concern(s).

Comment thread src/orb/ingest.ts
signatureHeader: string | null,
secret: string | undefined,
): Promise<boolean> {
return verifyGitHubSignature(body, signatureHeader, secret ?? "");

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.

P1: verifyOrbIngestSignature falls back to empty secret when ORB_INGEST_SECRET is undefined

verifyOrbIngestSignature passes secret ?? "" to verifyGitHubSignature. When ORB_INGEST_SECRET is not configured, an attacker can compute the request-body HMAC with an empty key and forge a valid signature.

Reject authentication when the secret is missing or empty, and make ORB_INGEST_SECRET required in src/env.d.ts.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/orb/ingest.ts">
<violation number="1" location="src/orb/ingest.ts:37">
<priority>P1</priority>
<title>verifyOrbIngestSignature falls back to empty secret when ORB_INGEST_SECRET is undefined</title>
<evidence>verifyOrbIngestSignature passes secret ?? "" to verifyGitHubSignature. When ORB_INGEST_SECRET is not configured, this falls back to an empty string secret. HMAC-SHA256 with an empty key produces a deterministic, easily computable signature, so an attacker can forge a valid x-orb-signature header for any request body.</evidence>
<recommendation>Reject authentication when the secret is missing or empty. Change verifyOrbIngestSignature to: if (!secret) return false; return verifyGitHubSignature(body, signatureHeader, secret); Also make ORB_INGEST_SECRET required in src/env.d.ts.</recommendation>
</violation>
</file>

@JSONbored

Copy link
Copy Markdown
Owner Author

Superseded by #1270-family work — see the registration-gate PR. We took your DoS hardening (the body-size cap + streaming read, now a 1 MiB ceiling → 413) but dropped the shared ORB_INGEST_SECRET requirement: it conflicts with #1257's hardwired-on telemetry (instances without the secret would 401, breaking "install the app, you're done"). Following das-github-mirror's actual model instead — open ingest + dedup + body limit + a registered=false-by-default instance gate, so only operator-registered instances count toward the fleet median. Reopen if you disagree with the trade-off.

@JSONbored JSONbored closed this Jun 24, 2026
@JSONbored
JSONbored deleted the codex/propose-fix-for-orb-ingest-vulnerability branch June 29, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:flagged PR flagged for review by security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Orb: fleet calibration collector + analytics (retire per-instance App; feed from review_audit)

1 participant