-
Notifications
You must be signed in to change notification settings - Fork 80
Feat/mdstream safe #280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat/mdstream safe #280
Conversation
WalkthroughThis update enhances XSS protection in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MarkdownRenderer
participant XSSSanitizer
participant UI
User->>MarkdownRenderer: Submit markdown with HTML
MarkdownRenderer->>XSSSanitizer: Sanitize HTML content
XSSSanitizer-->>MarkdownRenderer: Return sanitized HTML
MarkdownRenderer->>UI: Render sanitized content
User->>MarkdownRenderer: Submit markdown with link
MarkdownRenderer->>MarkdownRenderer: Validate URL (strip whitespace, check for "javascript")
alt URL is safe
MarkdownRenderer->>UI: Render link
else URL is unsafe
MarkdownRenderer->>UI: Render as plain text or block
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
gaoding.devingao seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-04-00.json (1)
5-5
: Improve the change description for clarity.The comment "修改g" appears incomplete or cryptic. Consider using a more descriptive message that explains what specific modification was made. Other change files in this PR use clearer descriptions like "解决html的xss问题" (solving HTML XSS problem).
- "comment": "修改g", + "comment": "修改HTML渲染安全性",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-02-57.json
(1 hunks)common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-04-00.json
(1 hunks)common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-07-01-12-49.json
(1 hunks)packages/chat-sdk/package.json
(3 hunks)packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts
(2 hunks)packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/html/index.tsx
(2 hunks)packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/link/index.tsx
(1 hunks)packages/chat-sdk/src/pages/markdown/const.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-04-00.json (2)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
Learnt from: huntye1
PR: coze-dev/coze-js#25
File: codecov.yml:4-8
Timestamp: 2024-11-12T09:27:06.509Z
Learning: In the `coze-js` project, when configuring `codecov.yml`, prefer using default settings initially for stability unless there's a specific need to change them.
packages/chat-sdk/package.json (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-07-01-12-49.json (2)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
Learnt from: huntye1
PR: coze-dev/coze-js#25
File: codecov.yml:4-8
Timestamp: 2024-11-12T09:27:06.509Z
Learning: In the `coze-js` project, when configuring `codecov.yml`, prefer using default settings initially for stability unless there's a specific need to change them.
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-02-57.json (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/html/index.tsx (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/link/index.tsx (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
packages/chat-sdk/src/pages/markdown/const.ts (1)
Learnt from: DingGao-Devin
PR: coze-dev/coze-js#104
File: packages/chat-sdk/src/pages/chat/index.tsx:37-41
Timestamp: 2025-02-24T03:34:17.077Z
Learning: The file `packages/chat-sdk/src/pages/chat/index.tsx` is a test implementation and doesn't require production-level improvements.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Node.js v20 (ubuntu-latest)
- GitHub Check: A job to sync events to Lark
🔇 Additional comments (9)
packages/chat-sdk/package.json (2)
3-3
: Version bump is appropriate for security improvements.The minor version increment is correctly applied for the XSS protection features being added to the package.
87-87
: No action needed: ‘xss’ is up-to-date and secureVersion 1.0.15 is the latest release (March 3, 2024) and has no known vulnerabilities according to both npm and Snyk assessments. You can safely retain this dependency as is.
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-07-01-12-49.json (1)
1-12
: Clear and descriptive change documentation.The change log entry properly documents the file URL modification with a clear, descriptive comment.
common/changes/@coze/chat-sdk/feat-mdstream-safe_2025-06-16-02-57.json (1)
1-12
: Excellent change documentation.The change log entry clearly describes the XSS security fix with a descriptive comment in Chinese. This is a good example of proper change documentation.
packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/html/index.tsx (2)
3-3
: XSS library import is correctly added.The import statement properly includes the xss library needed for HTML sanitization.
17-17
: Proper XSS sanitization implementation.The HTML content is correctly sanitized using
xss(node.value)
before being passed to theRichText
component. This effectively mitigates XSS attacks by removing dangerous HTML elements and attributes while preserving safe content.packages/chat-sdk/src/libs/ui-kit/atomic/md-stream/render/phrase/link/index.tsx (1)
22-22
: Excellent XSS protection enhancement!The improved URL validation effectively prevents javascript: URL bypass attempts by removing all whitespace and using case-insensitive matching. This addresses common XSS evasion techniques like
"java script:"
,"JAVASCRIPT:"
, or"java\tscript:"
.packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts (1)
64-66
: Good enhancement to include file URL in message content.Including both
file_id
andfile_url
provides complete file metadata for downstream consumers. The fallback toundefined
is appropriate for optional properties.packages/chat-sdk/src/pages/markdown/const.ts (1)
2-89
: Comprehensive XSS test coverage added.The extensive collection of XSS payloads provides excellent test coverage for various attack vectors including:
- Control character injection to bypass javascript: URL detection
- Event handler injection across multiple HTML elements
- Script tag variations with different encoding/whitespace
- Attribute and tag injection attempts
This robust test suite will help validate the effectiveness of the XSS protections implemented in other components.
// @ts-expect-error -- linter-disable-autofix | ||
packResult.file_url = res.url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Address TypeScript type issues instead of suppressing them.
The TypeScript linter disable suggests that the upload response type doesn't include a url
property. Consider updating the type definitions to properly reflect the API response structure.
- // @ts-expect-error -- linter-disable-autofix
- packResult.file_url = res.url;
+ packResult.file_url = res.url;
Update the upload response type definition to include the url
property, or verify that the API actually returns this field.
🏁 Script executed:
#!/bin/bash
# Description: Verify the upload response type definition includes url property
# Expected: Find type definitions for upload response to confirm url property
# Search for upload response type definitions
ast-grep --pattern 'interface $_ {
$$$
id: $_
$$$
}'
# Also search for upload method return types
rg -A 10 -B 5 "upload.*:" --type ts
Length of output: 56007
🏁 Script executed:
#!/bin/bash
# Search for FileResult type definitions
rg -R "interface FileResult" -A5 -B5 --type ts
Length of output: 453
🏁 Script executed:
#!/bin/bash
# Display context around the ts-expect-error in multi-send-message.ts
sed -n '120,160p' packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts
Length of output: 1304
🏁 Script executed:
#!/bin/bash
# Find FileResult definition and context in mini-coze-api
grep -R "FileResult" -n packages/chat-sdk/src/libs/utils/mini-coze-api.ts || true
sed -n '1,200p' packages/chat-sdk/src/libs/utils/mini-coze-api.ts
Length of output: 6304
Add the url
field to your FileObject type and remove the suppression
You’re suppressing a real TypeScript error because the upload response TypeScript definitions don’t include the URL property. To fix:
• packages/coze-js/src/resources/files/files.ts
– Extend FileObject
with the url
field to match what the API actually returns.
export interface FileObject {
/** The ID of the uploaded file. */
id: string;
/** The total number of bytes in the file. */
bytes: number;
/** The upload time, formatted as a 10-digit Unix timestamp in seconds. */
created_at: number;
/** The name of the file. */
file_name: string;
+ /** Public URL of the uploaded file. */
+ url: string;
}
• packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts
– Remove the @ts-expect-error
and re-enable the assignment once the type is correct.
- // @ts-expect-error -- linter-disable-autofix
- packResult.file_url = res.url;
+ packResult.file_url = res.url;
After updating the type, run your build to verify the error is gone and the field is available at runtime.
🤖 Prompt for AI Agents
In packages/coze-js/src/resources/files/files.ts, extend the FileObject type to
include the url field to reflect the actual API response. Then, in
packages/chat-sdk/src/libs/services/helper/message/send-message/multi-send-message.ts
at lines 138-139, remove the @ts-expect-error comment and keep the assignment of
packResult.file_url = res.url. Finally, run the build to confirm the TypeScript
error is resolved and the url field is properly recognized.
Summary by CodeRabbit
New Features
Bug Fixes
Tests