Skip to content

fix(data-api): cap direct chain analytics limits#4900

Open
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-unbounded-limits-in-data-api
Open

fix(data-api): cap direct chain analytics limits#4900
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-unbounded-limits-in-data-api

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent unauthenticated callers from bypassing the main Worker’s 1..100 page-size cap and binding huge limit values into Postgres LIMIT clauses, which can amplify DB work and cause availability/cost issues.
  • Ensure the Postgres-serving Data Worker remains service-binding-only and not accidentally reachable via workers.dev preview URLs.

Description

  • Clamp chain-wide analytics limit values in the data Worker by updating chainLimit to call clampRequestLimit(..., { defaultLimit, maxLimit: 100 }) so SQL LIMIT parameters are bounded at 100 in workers/data-api.mjs.
  • Disable public preview and workers.dev exposure for the data Worker by adding `

Codex Task

@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 11, 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
metagraphed-data-api 047200b Commit Preview URL

Branch Preview URL
Jul 11 2026, 03:11 PM

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.85%. Comparing base (cc0db0e) to head (047200b).
⚠️ Report is 13 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4900      +/-   ##
==========================================
- Coverage   97.85%   97.85%   -0.01%     
==========================================
  Files         162      162              
  Lines       19746    19745       -1     
  Branches     7419     7418       -1     
==========================================
- Hits        19323    19322       -1     
  Misses         59       59              
  Partials      364      364              
Files with missing lines Coverage Δ
workers/data-api.mjs 99.76% <100.00%> (-0.01%) ⬇️
🚀 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 Jul 11, 2026
@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix or unsolicited PR — scores a 0.05x multiplier. label Jul 11, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 17:10:33 UTC

3 files · 1 AI reviewer · 2 blockers · readiness 80/100 · CI green · dirty

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR hardens the Postgres-serving data Worker so it fails safe if ever reached directly: chainLimit now routes through clampRequestLimit(..., {defaultLimit, maxLimit: 100}) instead of doing a bare Number(raw) cast, closing the path where an unbounded ?limit= value would bind straight into a SQL LIMIT clause, and wrangler.data.jsonc disables workers_dev/preview_urls so the Worker isn't reachable outside the service binding. Two new tests confirm limit=1000000000 is clamped to 100 in the emitted SQL parameters for both /chain/transfers and /chain/transfer-pairs. The change is narrowly scoped, the security rationale (defense-in-depth against a bypassed page-size cap) is sound, and CI is green.

Nits — 5 non-blocking
  • chainLimit's call site isn't shown in the diff, so it's worth double-checking every call to chainLimit() still passes only (url, defaultLimit) and that clampRequestLimit is already imported/defined elsewhere in workers/data-api.mjs — the diff itself doesn't show that import.
  • The old comment explained why chainLimit could skip re-validation (D1-side parseLimitParam already validated); the new comment is good but doesn't mention what happens on invalid/non-numeric limit input (e.g. NaN) — worth confirming clampRequestLimit's own docstring/behavior covers that, since it's not visible in this diff.
  • Consider adding a regression test for a negative or non-numeric ?limit= value (e.g. limit=-5 or limit=abc) hitting chainLimit directly, to confirm clampRequestLimit's fallback-to-defaultLimit behavior is exercised, not just the upper-bound clamp.
  • The wrangler.data.jsonc comment could reference the specific issue/threat (service-binding-only) so future editors understand why workers_dev/preview_urls must stay false.
  • tests/data-api.test.mjs:4424 and tests/data-api.test.mjs:4495 only cover oversized limits; add one direct-worker regression for `limit=0` or a malformed value if `clampRequestLimit` is meant to preserve the full public `1..100` behavior here.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ⚠️ 12/25 Preflight needs author follow-up before maintainer review.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 37 merged, 414 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 414 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 45 PR(s), 414 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Address findings or add validation evidence.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix or unsolicited PR — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant