Skip to content

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

Open
fairlighteth wants to merge 4 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 4 commits into
developfrom
deepsec/medium-03-cowfi-content-token-trust

Conversation

@fairlighteth

@fairlighteth fairlighteth commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses the remaining smaller cow-fi MEDIUM findings around external content links and remote token payload trust.

This PR keeps the token pages working, but stops trusting malformed remote token data:

  • Upgrades the Balancer FAQ link to HTTPS.
  • Adds runtime validation for remote token payloads before they are used to build token pages and swap-link metadata.
  • Rejects malformed platform/address data instead of flowing it into token-derived links.
  • Preserves valid token pages while degrading invalid optional metrics to null.
  • Adds focused token validation tests for malformed remote payloads.

To Test

  1. Run targeted cow-fi checks
  • NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:test --runInBand passes.
  • NX_DAEMON=false ./node_modules/.bin/nx run cow-fi:lint --verbose completes with 0 errors.
  1. Verify the CoW AMM content link
  • Open the CoW AMM FAQ/content that links to Balancer.
  • Confirm the Balancer link uses https:// rather than http://.
  1. Verify token page behavior with valid data
  • Open a normal token page under /tokens/[tokenId] and confirm metadata and swap-link data still render.
  • Confirm valid platform addresses still produce the expected token metadata and routing behavior.
  1. Verify malformed remote token data fails closed
  • With a mocked or test payload, confirm malformed contract addresses are ignored instead of trusted.
  • Confirm invalid optional market data falls back to null/unavailable state rather than crashing the page.
  • Confirm tokens that fail the required shape checks are skipped instead of producing broken swap links.

Background

The report grouped these items together because they were all content-trust issues in cow-fi rather than app-flow issues.

The biggest part is the token-content boundary: remote token JSON is still allowed, but only after its required fields and platform addresses pass validation.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced robustness of token price and market data display with graceful fallbacks when data is unavailable
    • Improved 24-hour change display with better default values when data is missing
    • Updated external links to use HTTPS for enhanced security

@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 9, 2026 1:01pm
explorer-dev Ready Ready Preview Jun 9, 2026 1:01pm
storybook Ready Ready Preview Jun 9, 2026 1:01pm
swap-dev Ready Ready Preview Jun 9, 2026 1:01pm
widget-configurator Ready Ready Preview Jun 9, 2026 1:01pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored Jun 9, 2026 1:01pm
sdk-tools Ignored Ignored Preview Jun 9, 2026 1:01pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2d459469-6e94-4968-9102-86aa3be9eb6c

📥 Commits

Reviewing files that changed from the base of the PR and between 9c7abf2 and 14e6078.

📒 Files selected for processing (9)
  • apps/cow-fi/app/(main)/tokens/[tokenId]/page.tsx
  • apps/cow-fi/components/ChartSection/index.test.tsx
  • apps/cow-fi/components/ChartSection/index.tsx
  • apps/cow-fi/data/cow-amm/const.tsx
  • apps/cow-fi/services/tokens/index.ts
  • apps/cow-fi/services/tokens/validation.test.ts
  • apps/cow-fi/services/tokens/validation.ts
  • apps/cow-fi/types/index.ts
  • apps/cow-fi/util/getPriceChangeColor.ts

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.
Description check ✅ Passed The PR description follows the required template structure with Summary, To Test, and Background sections. It clearly explains the changes, provides detailed testing instructions with checkboxes, and includes helpful background context.
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.

✏️ 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: 15c5be2
Status: ✅  Deploy successful!
Preview URL: https://460a275e.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: 15c5be2
Status: ✅  Deploy successful!
Preview URL: https://849807dd.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