Skip to content

fix: tighten cow-fi content links and remote token parsing#7619

Open
fairlighteth wants to merge 7 commits into
developfrom
deepsec/medium-03-cowfi-content-token-trust
Open

fix: tighten cow-fi content links and remote token parsing#7619
fairlighteth wants to merge 7 commits into
developfrom
deepsec/medium-03-cowfi-content-token-trust

Conversation

@fairlighteth

@fairlighteth fairlighteth commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Updated the CoW AMM Balancer link to use https://.
  • Added runtime validation for remote cow.fi token payloads before building token pages and swap metadata.
  • Drops malformed platform entries, including invalid contract addresses.
  • Keeps valid token pages rendering when optional market metrics are missing.
  • Skips the Ethereum chart query when a token has no valid Ethereum platform.

Why

  • Remote token JSON should not flow directly into token pages, chart queries, or swap-link metadata without shape and address validation.
  • Invalid optional market data should degrade to unavailable values instead of crashing or producing broken metadata.

Validation

  • apps/cow-fi/services/tokens/validation.test.ts covers required token fields, malformed platform entries, invalid contract addresses, and market-cap rank normalization.
  • apps/cow-fi/components/ChartSection/index.test.tsx covers the missing-Ethereum-platform case.
  • Targeted checks:
    • NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:test --runInBand
    • NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:lint --verbose
  • Manual check: open the CoW AMM FAQ/content and confirm the Balancer link uses https://.

@vercel

vercel Bot commented Jun 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cowfi Ready Ready Preview Jun 16, 2026 6:03pm
explorer-dev Ready Ready Preview Jun 16, 2026 6:03pm
storybook Ready Ready Preview Jun 16, 2026 6:03pm
swap-dev Ready Ready Preview Jun 16, 2026 6:03pm
widget-configurator Ready Ready Preview Jun 16, 2026 6:03pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored Jun 16, 2026 6:03pm
sdk-tools Ignored Ignored Preview Jun 16, 2026 6:03pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR transitions token numeric market fields from string to number types throughout the entire data pipeline: type definitions, a new validation/normalization layer, token service integration, and consuming utilities and components. All changes add defensive runtime validation and type safety for price, volume, market cap, and related metrics.

Changes

Token Numeric Type Safety Initiative

Layer / File(s) Summary
Type definitions for numeric market fields
apps/cow-fi/types/index.ts
TokenInfo and TokenDetails interfaces convert priceUsd, change24h, volume, marketCap, allTimeHigh, and allTimeLow from string | null to number | null.
Validation and normalization infrastructure
apps/cow-fi/services/tokens/validation.ts, apps/cow-fi/services/tokens/validation.test.ts
New module exports runtime validators: isRawTokenData enforces required string fields, normalizePlatformData validates EVM addresses with decimal fallback, normalizePlatforms builds platform mappings, and metric normalizers convert external values to numbers or null. Tests confirm address and rank validation behavior.
Token service integration and normalization
apps/cow-fi/services/tokens/index.ts
Service refactors to treat payloads as unknown, validate array shape at runtime, and map only entries passing isRawTokenData. _toTokenDetails normalizes all numeric fields and platforms via helpers. COW token reordering and fetch signature updated to support the defensive pipeline.
Utility function updates for numeric types
apps/cow-fi/util/getPriceChangeColor.ts
Function signature accepts number | string | null; explicit null and NaN handling replaces truthiness checks; theme color logic preserved for positive/negative numeric values.
Components and pages consuming new types
apps/cow-fi/components/ChartSection/index.tsx, apps/cow-fi/components/ChartSection/index.test.tsx, apps/cow-fi/app/(main)/tokens/[tokenId]/page.tsx
ChartSection uses optional chaining for Ethereum address and explicit return typing; test verifies query-skip when platform absent. Token page metadata conditionally formats price and change only when numeric.
Security updates
apps/cow-fi/data/cow-amm/const.tsx
Balancer pools FAQ link upgraded from HTTP to HTTPS.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • elena-zh
  • shoom3301

Poem

A rabbit hops through numbers clean,
From strings to types of numeric sheen—
With validation guards and defaults sound,
Safe prices in each token found. 🐰💚

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: tighten cow-fi content links and remote token parsing' accurately reflects the main changes: hardening external content links (HTTPS upgrade) and adding validation for remote token parsing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly explains what changed, why it was changed, and provides specific validation steps and testing commands.

✏️ 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 deepsec/medium-03-cowfi-content-token-trust

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.

@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying explorer-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 142abb3
Status: ✅  Deploy successful!
Preview URL: https://b91fe132.explorer-dev-dxz.pages.dev
Branch Preview URL: https://deepsec-medium-03-cowfi-cont.explorer-dev-dxz.pages.dev

View logs

@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying swap-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 142abb3
Status: ✅  Deploy successful!
Preview URL: https://d6cd5343.swap-dev-5u6.pages.dev
Branch Preview URL: https://deepsec-medium-03-cowfi-cont.swap-dev-5u6.pages.dev

View logs

fairlighteth

This comment was marked as outdated.

@fairlighteth fairlighteth left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

⚠️ AI Review (Codex GPT-5, worked 3m 59s): invalid Ethereum platform now crashes token pages

Finding: Invalid or missing ethereum platform data now crashes the token page

  • Location: apps/cow-fi/components/ChartSection/index.tsx:28
  • This PR correctly drops malformed platform entries in normalizePlatforms(), but ChartSection still assumes platforms.ethereum always exists.
  • If a remote token payload has an invalid/missing detail_platforms.ethereum entry, platforms.ethereum becomes undefined, and platforms.ethereum.contractAddress throws before the chart can fall back.
  • Impact: the token page no longer fails closed for the malformed payloads this PR is trying to harden against.

Suggested fix

  • Guard the chart path with optional chaining or a local const ethereumAddress = platforms.ethereum?.contractAddress so missing Ethereum data skips the query instead of crashing.
  • Add a focused test for a token whose Ethereum platform is invalid/missing while another platform remains valid.
Review scope and related context

This is separate from existing review comments, which currently only contain deploy/bot messages. I also checked the other platform consumers here: SwapWidget and the swap-link cards already use optional access, so this assumption appears isolated to ChartSection.

🤖 Prompt for AI agents
Verify this finding against current code. Fix only if still valid, keep the change minimal, and validate with the targeted tests.

Context:
- apps/cow-fi/components/ChartSection/index.tsx:28
- normalizePlatforms() now removes malformed ethereum entries instead of preserving a placeholder object
- platforms.ethereum.contractAddress throws when ethereum is missing
- expected fix: skip the chart query when ethereum is absent, and add coverage for invalid/missing ethereum platform data

- skip the Ethereum price query when a token has no valid Ethereum
  platform entry
- add regression coverage for token pages that keep non-Ethereum
  platforms after validation

@fairlighteth fairlighteth left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AI Review (Codex GPT-5, worked 1m 10s): follow-up addressed

Rechecked

  • Author response: added fix: avoid cow-fi chart crashes on invalid token platforms in 14e60784ba9debb9949e179e1b2320279e64d8fe.
  • Code path: apps/cow-fi/components/ChartSection/index.tsx:26.
  • Test coverage: apps/cow-fi/components/ChartSection/index.test.tsx and NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:test --runInBand --testPathPatterns=apps/cow-fi/components/ChartSection/index.test.tsx.

Result: Fixed. ChartSection now reads platforms.ethereum?.contractAddress through a guarded local variable, so missing/invalid Ethereum platform data skips the query instead of throwing, and the new test covers that malformed-platform case.

🤖 Verification notes for AI agents
Verify the prior finding against current code only. Confirm whether the author's claimed fix addresses the specific failure mode, and avoid reopening broader or unrelated issues.

Prior finding:
- ChartSection crashed when normalizePlatforms() dropped an invalid ethereum entry and the code still dereferenced platforms.ethereum.contractAddress.

Current verification:
- ChartSection now guards ethereumAddress via optional access before building queryVariables.
- Added a focused test that renders with only a base platform and asserts the component does not throw and skips the query.

@fairlighteth

Copy link
Copy Markdown
Contributor Author
PR - Automated QA (Codex GPT-5, worked 8m): token-page and CoW AMM content flows verified on the PR preview

Outcome

  • Passed: Firefox 141.0 loaded /tokens/cow-protocol on the PR preview with token title, chart/stats, and token content rendering normally.
  • Passed: Firefox 141.0 refresh on /tokens/cow-protocol remained stable.
  • Passed: Firefox 141.0 redirected /tokens/not-a-real-token to /tokens.
  • Passed: Firefox 141.0 verified the CoW AMM Balancer CTA uses https://balancer.fi/pools/cow.
  • Passed: Chromium 140.0.7339.16 loaded /tokens/cow-protocol successfully after Playwright locale normalization to en-US.
  • Passed: Local targeted tests for malformed token validation and the missing-Ethereum-platform chart guard.

Source under test

  • Browser-based verification was performed against the deployed branch preview built from PR head 14e60784ba9debb9949e179e1b2320279e64d8fe.
  • Local worktree matched the same commit, and targeted local tests ran on that branch.
  • Results were visually inspected from captured screenshots.

Environment:

  • Browsers: Chromium 140.0.7339.16 and Firefox 141.0
  • OS: Linux

Wallet state reached:

  • disconnected: yes
  • provider-injected: no
  • wallet-connected-ui: no
  • signing-capable: no

If re-running locally:

  • Frontend: pnpm start:cowfi
  • Route: /tokens/cow-protocol, /tokens/not-a-real-token, /cow-amm
  • Local state required: none
  • Fixtures/mocks: none for the browser pass; malformed server-side token payload coverage would need a local fixture/proxy for the CDN response

Expected results:

  1. Open /tokens/cow-protocol
    Expected: token page renders normally with title, chart/stats section, and token content; no NaN/undefined metadata artifacts.
  2. Refresh /tokens/cow-protocol
    Expected: page remains stable after reload.
  3. Open /tokens/not-a-real-token
    Expected: redirect to /tokens.
  4. Open /cow-amm
    Expected: Balancer CTA resolves to https://balancer.fi/pools/cow.

Browser coverage:

  • Chromium 140.0.7339.16: /tokens/cow-protocol verified after locale normalization to en-US.
  • Firefox 141.0: valid token page load, refresh, invalid-token redirect, and CoW AMM HTTPS link verified.

Why these adjacent flows were included:

  • Invalid-token redirect shares the same token-page route/container boundary touched by the PR.
  • Refresh checks that token-page consumers remain stable after initial render.
  • CoW AMM link verification is the direct user-visible content-link change called out by the PR.
Note: Playwright Chromium locale normalization

Impact:

  • This did not undercut the PR claim, but it mattered for reliable Chromium QA on this machine.

Observed behavior:

  • An initial headless Chromium run inherited a bad locale shape and crashed before render with RangeError: Invalid language tag: en-US@posix.
  • Re-running Chromium with locale normalization (--lang=en-US and context locale: 'en-US') rendered the token page successfully.

Interpretation:

  • This was an environment artifact in the headless Playwright setup, not a PR-specific browser regression.

Commands run:

  • PLAYWRIGHT_BROWSERS_PATH=$HOME/.cache/ms-playwright-local node ...playwright... against https://cowfi-git-deepsec-medium-03-cowfi-content-token-trust-cowswap.vercel.app
  • NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:test --runInBand --testPathPatterns='apps/cow-fi/services/tokens/validation.test.ts|apps/cow-fi/components/ChartSection/index.test.tsx'

Environment normalization used:

  • Chromium launch arg --lang=en-US
  • Playwright browser context locale: 'en-US'

Still manual if desired:

  • Human visual spot-check in a normal non-headless Chrome/Brave session
  • End-to-end browser proof of malformed server-side token payload behavior with a controlled CDN fixture/proxy

Artifacts:

  • Chromium token page:
    Chromium token page success for PR 7619
  • Firefox token page:
    Firefox token page success for PR 7619
  • Firefox invalid token redirect:
    Firefox invalid token redirect for PR 7619
  • Firefox CoW AMM Balancer link:
    Firefox CoW AMM Balancer link for PR 7619

Residual gaps:

  • Malformed server-side token payload was not injected in-browser end to end during this run.
  • Chromium was only used for the core token-page success check in this rerun, not the full Firefox scenario set.
  • Wallet-connected coverage was not relevant to the touched flows.

@fairlighteth fairlighteth marked this pull request as ready for review June 8, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants