Skip to content

fix: resolve mutable default argument in validateFrameMessage - #556

Open
dajiaohuang wants to merge 6 commits into
framesjs:mainfrom
dajiaohuang:fix/mutable-default-argument-validateFrameMessage
Open

fix: resolve mutable default argument in validateFrameMessage#556
dajiaohuang wants to merge 6 commits into
framesjs:mainfrom
dajiaohuang:fix/mutable-default-argument-validateFrameMessage

Conversation

@dajiaohuang

Copy link
Copy Markdown

Summary

修复 validateFrameMessage 函数中的可变默认参数问题。

问题

  1. 原代码使用了可变对象作为默认参数
  2. 原代码中 api_key header 没有被添加到请求中(当使用默认 hub URL 时)

Fix

  • 使用空对象作为 hubRequestOptions 默认值
  • 在请求中明确添加 api_key: DEFAULT_HUB_API_KEY

- Remove redundant else branch that was overwriting acc[type] with the same value
- Fix mutable default argument by creating requestOptions object fresh each time
- Properly merge user-provided hubRequestOptions with default api_key header
Create fresh requestOptions object each time to avoid shared mutable default.
Properly merge user-provided hubRequestOptions with default api_key header.
Create fresh requestOptions object each time to avoid shared mutable default.
Properly merge user-provided hubRequestOptions with default api_key header.
- Fix mutable default argument by using empty object as default
- Add missing api_key header when using default hub URL
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

@dajiaohuang is attempting to deploy a commit to the Discove Team on Vercel.

A member of the Team first needs to authorize it.

Comment thread packages/frames.js/src/getUserDataForFid.ts
Comment on lines +22 to +30
const requestHeaders = {
...(hubRequestOptions.headers ?? {}),
api_key: DEFAULT_HUB_API_KEY,
};

const requestOptions = {
...hubRequestOptions,
headers: requestHeaders,
};

@vercel vercel Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getUserDataForFid unconditionally injects Neynar's public api_key header even for custom (non-Neynar) hubs, leaking the default key contrary to the intended design used elsewhere in the codebase.

Fix on Vercel

Comment on lines +30 to +31
...(hubRequestOptions.headers ?? {}),
api_key: DEFAULT_HUB_API_KEY,

@vercel vercel Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getAddressForFid unconditionally injects Neynar's public DEFAULT_HUB_API_KEY into request headers after the caller's headers, overriding any user-supplied api_key and leaking the Neynar key to custom (non-Neynar) hubs.

Fix on Vercel

Comment on lines +40 to +41
...(hubRequestOptions.headers ?? {}),
api_key: DEFAULT_HUB_API_KEY,

@vercel vercel Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getAddressesForFid forces api_key: DEFAULT_HUB_API_KEY after spreading user headers, overriding any caller-supplied api_key and leaking Neynar's public key to custom hubs.

Fix on Vercel

Comment on lines 53 to +54
...headers,
api_key: DEFAULT_HUB_API_KEY,

@vercel vercel Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

validateFrameMessage unconditionally sends Neynar's public api_key and overrides any user-supplied api_key header, breaking auth against custom hubs.

Fix on Vercel

@dajiaohuang

Copy link
Copy Markdown
Author

Addressed the current review feedback in the latest commit. The user-data reducer now stores the first value for each type and replaces it only when a newer timestamp is seen, so first-seen fields are no longer lost. Validation: git diff --check passed. The local checkout has no Yarn binary/dependencies, so the package test suite could not run here.

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.

1 participant