Merged
Conversation
📝 WalkthroughWalkthroughThis change introduces a comprehensive Feishu-to-Codes Assistant bridge system, including deployment automation, service orchestration, and a full-featured Node.js bridge implementation with message normalization, media handling, and HTTP API integration for production deployment. Changes
Sequence DiagramsequenceDiagram
participant Feishu as Feishu User/App
participant Bridge as Feishu Bridge
participant Codes as Codes Assistant API
participant Media as Media Storage
Feishu->>Bridge: WebSocket Message (text/media)
Bridge->>Bridge: Parse & normalize message
alt Has Media
Bridge->>Media: Download image/video/audio
Media-->>Bridge: Media content
Bridge->>Bridge: Convert to data URL/path
end
Bridge->>Codes: POST /askAssistant (text + attachments)
Codes->>Codes: Process with context
Codes-->>Bridge: Response text + media lines
Bridge->>Bridge: Extract & validate media URLs
alt Response has Media
Bridge->>Media: Fetch/convert media
Media-->>Bridge: Media content
Bridge->>Feishu: Upload & send media
end
Bridge->>Feishu: Send reply message
Feishu-->>Feishu User/App: Display response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
bridge/bridge.mjs (1)
476-488: Consider adding a size limit to the deduplication cache.The
seenMap grows unbounded if cleanup doesn't keep pace with incoming messages. Under high load, this could cause memory pressure.🔧 Proposed fix with size limit
const seen = new Map(); const SEEN_TTL_MS = 10 * 60 * 1000; +const SEEN_MAX_SIZE = 10000; function isDuplicate(messageId) { const now = Date.now(); + // Cleanup expired and enforce size limit for (const [k, ts] of seen) { if (now - ts > SEEN_TTL_MS) seen.delete(k); } + if (seen.size > SEEN_MAX_SIZE) { + // Remove oldest 20% + const toRemove = [...seen.keys()].slice(0, Math.floor(SEEN_MAX_SIZE * 0.2)); + for (const k of toRemove) seen.delete(k); + } if (!messageId) return false; if (seen.has(messageId)) return true; seen.set(messageId, now); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bridge/bridge.mjs` around lines 476 - 488, The deduplication Map (seen) can grow unbounded; update isDuplicate (and related constants) to enforce a maximum cache size (e.g., SEEN_MAX_ENTRIES) and evict oldest entries when the Map exceeds that size during insert/cleanup; keep the existing TTL cleanup using SEEN_TTL_MS but after deleting expired entries, if seen.size >= SEEN_MAX_ENTRIES remove oldest keys (iterate seen.keys() or entries() to delete until under limit) before calling seen.set(messageId, now) to prevent memory pressure.DEPLOY.md (1)
70-75: Add language specifier to fenced code block.Per the static analysis hint, this code block should have a language specified for proper syntax highlighting.
📝 Proposed fix
-``` +```bash # 本机执行 GOOS=linux GOARCH=amd64 go build -o codes-linux ./cmd/codes scp codes-linux user@server:/usr/local/bin/codes</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@DEPLOY.mdaround lines 70 - 75, Update the fenced code block in DEPLOY.md to
include a language specifier (e.g., "bash") so the block renders with proper
syntax highlighting; locate the triple-backtick block containing the GOOS/GOARCH
build and scp commands and change the opening fence fromtobash (keeping
the exact commands unchanged).</details> </blockquote></details> <details> <summary>deploy.sh (1)</summary><blockquote> `114-116`: **Quote glob pattern in `GONOSUMDB` assignment.** The static analyzer flagged that `GONOSUMDB=*` should be quoted to prevent shell glob expansion, though in this assignment context it's typically safe. <details> <summary>🔧 Proposed fix</summary> ```diff # 设置 Go 模块代理（国内服务器必需，否则 github.com 依赖下载极慢/超时） export GOPROXY=https://goproxy.cn,https://goproxy.io,direct -export GONOSUMDB=* +export GONOSUMDB='*' info "使用 Go 模块代理: $GOPROXY" ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@deploy.sh` around lines 114 - 116, The GONOSUMDB assignment uses an unquoted glob (*) which can be expanded by the shell; update the export of GONOSUMDB in deploy.sh (the line that sets GONOSUMDB=*) to quote the glob (e.g., GONOSUMDB="*") so the literal asterisk is assigned, preventing unintended filename expansion while keeping the rest of the export lines unchanged. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@bridge/bridge.mjs:
- Around line 205-218: The path allowlist check is vulnerable to symlink escape
because path.resolve() doesn't follow symlinks; update isPathInside,
isAllowedLocalPath, and isAllowedOutboundPath to compare
fs.realpathSync-resolved paths instead of raw resolved paths: call
fs.realpathSync (with try/catch to handle missing paths) on both the child and
parent paths before using path.relative or equality checks, and use those real
paths in isPathInside and the ALLOWED_* checks to prevent symlink bypasses.- Around line 382-409: downloadUrlToTempFile currently follows redirects
recursively without limits, lacks request timeouts, and accepts redirects that
can enable SSRF; update downloadUrlToTempFile to accept an optional
redirectCount (default 0) and enforce a maxRedirects constant (e.g., 5) so any
redirect increments redirectCount and rejects if exceeded; before following a
redirect (the loc from res.headers.location) parse it with new URL() and
validate the hostname/ip against a safe allowlist or reject private/reserved IP
ranges to mitigate SSRF; add a request timeout using AbortController or
req.setTimeout and ensure the request is aborted and promise rejected on
timeout; keep existing behavior for writing to tmp and propagate errors
correctly.In
@bridge/setup-service.mjs:
- Around line 28-68: The plist string interpolates user-controlled values (e.g.,
APP_ID, SECRET_PATH, HOME and any other ${...} like LABEL, NODE_PATH,
BRIDGE_PATH, WORK_DIR) directly which can break XML; create and call a small
XML-escaping helper (e.g., escapeXml) to replace &, <, >, " and ' in those
variables and use the escaped results when building the plist template so the
generated plist remains well-formed and safe.- Around line 23-24: The code uses import.meta.dirname (in BRIDGE_PATH and
WORK_DIR) which requires Node 20.11+, so replace that usage with the Node
18-compatible pattern: compute the current module file path from import.meta.url
(via fileURLToPath) and then use path.dirname to derive the directory, and use
that directory to build BRIDGE_PATH and WORK_DIR; update any imports to ensure
fileURLToPath is imported from 'url' and remove import.meta.dirname references
in BRIDGE_PATH and WORK_DIR.In
@DEPLOY.md:
- Around line 482-493: In the "安全注意事项" list within DEPLOY.md there are two items
labeled "4."; update the second duplicated "4." (the line starting "所有
/assistant请求都需要 Bearer Token 认证") to "5." so the ordered list numbers are
sequential; locate the list under the "安全注意事项" heading and change the numbering
for that final item from 4. to 5.
Nitpick comments:
In@bridge/bridge.mjs:
- Around line 476-488: The deduplication Map (seen) can grow unbounded; update
isDuplicate (and related constants) to enforce a maximum cache size (e.g.,
SEEN_MAX_ENTRIES) and evict oldest entries when the Map exceeds that size during
insert/cleanup; keep the existing TTL cleanup using SEEN_TTL_MS but after
deleting expired entries, if seen.size >= SEEN_MAX_ENTRIES remove oldest keys
(iterate seen.keys() or entries() to delete until under limit) before calling
seen.set(messageId, now) to prevent memory pressure.In
@DEPLOY.md:
- Around line 70-75: Update the fenced code block in DEPLOY.md to include a
language specifier (e.g., "bash") so the block renders with proper syntax
highlighting; locate the triple-backtick block containing the GOOS/GOARCH build
and scp commands and change the opening fence fromtobash (keeping the
exact commands unchanged).In
@deploy.sh:
- Around line 114-116: The GONOSUMDB assignment uses an unquoted glob () which
can be expanded by the shell; update the export of GONOSUMDB in deploy.sh (the
line that sets GONOSUMDB=) to quote the glob (e.g., GONOSUMDB="*") so the
literal asterisk is assigned, preventing unintended filename expansion while
keeping the rest of the export lines unchanged.</details> --- <details> <summary>ℹ️ Review info</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 432f7ee4f58b7f80b82bad24381669c80da126fd and 575a7e514f26d0c249244e9e613c5be4ccfd1d62. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `bridge/package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (8)</summary> * `.gitignore` * `.mcp.json` * `DEPLOY.md` * `bridge/.env.example` * `bridge/bridge.mjs` * `bridge/package.json` * `bridge/setup-service.mjs` * `deploy.sh` </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * .mcp.json </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+205
to
+218
| function isPathInside(child, parent) { | ||
| const rel = path.relative(parent, child); | ||
| return !!rel && !rel.startsWith('..') && !path.isAbsolute(rel); | ||
| } | ||
|
|
||
| function isAllowedLocalPath(filePath) { | ||
| const p = path.resolve(filePath); | ||
| return ALLOWED_LOCAL_MEDIA_DIRS.some((dir) => isPathInside(p, dir) || p === dir); | ||
| } | ||
|
|
||
| function isAllowedOutboundPath(filePath) { | ||
| const p = path.resolve(filePath); | ||
| return ALLOWED_OUTBOUND_MEDIA_DIRS.some((dir) => isPathInside(p, dir) || p === dir); | ||
| } |
There was a problem hiding this comment.
Symlink escape risk in path allowlist checks.
path.resolve() does not resolve symlinks. An attacker with write access could create a symlink inside an allowed directory pointing outside it, bypassing the allowlist.
🛡️ Proposed fix using fs.realpathSync
function isPathInside(child, parent) {
- const rel = path.relative(parent, child);
+ let resolvedChild, resolvedParent;
+ try {
+ resolvedChild = fs.realpathSync(child);
+ resolvedParent = fs.realpathSync(parent);
+ } catch {
+ return false; // Path doesn't exist or can't be resolved
+ }
+ const rel = path.relative(resolvedParent, resolvedChild);
return !!rel && !rel.startsWith('..') && !path.isAbsolute(rel);
}
function isAllowedLocalPath(filePath) {
- const p = path.resolve(filePath);
- return ALLOWED_LOCAL_MEDIA_DIRS.some((dir) => isPathInside(p, dir) || p === dir);
+ try {
+ const p = fs.realpathSync(filePath);
+ return ALLOWED_LOCAL_MEDIA_DIRS.some((dir) => {
+ try {
+ const d = fs.realpathSync(dir);
+ return isPathInside(p, d) || p === d;
+ } catch { return false; }
+ });
+ } catch { return false; }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bridge/bridge.mjs` around lines 205 - 218, The path allowlist check is
vulnerable to symlink escape because path.resolve() doesn't follow symlinks;
update isPathInside, isAllowedLocalPath, and isAllowedOutboundPath to compare
fs.realpathSync-resolved paths instead of raw resolved paths: call
fs.realpathSync (with try/catch to handle missing paths) on both the child and
parent paths before using path.relative or equality checks, and use those real
paths in isPathInside and the ALLOWED_* checks to prevent symlink bypasses.
Comment on lines
+382
to
+409
| async function downloadUrlToTempFile(url) { | ||
| const u = String(url); | ||
| const ext = extLower(u) || 'bin'; | ||
| const tmp = path.join(os.tmpdir(), `feishu_bridge_${Date.now()}_${Math.random().toString(16).slice(2)}.${ext}`); | ||
|
|
||
| const proto = u.startsWith('https') ? https : http; | ||
|
|
||
| await new Promise((resolve, reject) => { | ||
| const req = proto.get(u, (res) => { | ||
| if (res.statusCode === 301 || res.statusCode === 302) { | ||
| const loc = res.headers.location; | ||
| res.resume(); | ||
| if (!loc) return reject(new Error('Redirect without location header')); | ||
| downloadUrlToTempFile(loc).then(resolve).catch(reject); | ||
| return; | ||
| } | ||
| if (res.statusCode !== 200) { | ||
| res.resume(); | ||
| return reject(new Error(`HTTP ${res.statusCode}`)); | ||
| } | ||
| const out = fs.createWriteStream(tmp); | ||
| pipeline(res, out).then(resolve).catch(reject); | ||
| }); | ||
| req.on('error', reject); | ||
| }); | ||
|
|
||
| return tmp; | ||
| } |
There was a problem hiding this comment.
Unbounded redirect following and missing timeout create reliability and security risks.
- Infinite redirect loop: The recursive redirect handling has no depth limit.
- SSRF risk: Following redirects could lead to internal services.
- No timeout: Requests could hang indefinitely on slow/unresponsive servers.
🛡️ Proposed fix with redirect limit and timeout
-async function downloadUrlToTempFile(url) {
+async function downloadUrlToTempFile(url, redirectCount = 0) {
+ const MAX_REDIRECTS = 5;
+ const TIMEOUT_MS = 30000;
+
+ if (redirectCount > MAX_REDIRECTS) {
+ throw new Error(`Too many redirects (>${MAX_REDIRECTS})`);
+ }
+
const u = String(url);
const ext = extLower(u) || 'bin';
const tmp = path.join(os.tmpdir(), `feishu_bridge_${Date.now()}_${Math.random().toString(16).slice(2)}.${ext}`);
const proto = u.startsWith('https') ? https : http;
await new Promise((resolve, reject) => {
- const req = proto.get(u, (res) => {
+ const req = proto.get(u, { timeout: TIMEOUT_MS }, (res) => {
if (res.statusCode === 301 || res.statusCode === 302) {
const loc = res.headers.location;
res.resume();
if (!loc) return reject(new Error('Redirect without location header'));
- downloadUrlToTempFile(loc).then(resolve).catch(reject);
+ // Prevent redirect to internal IPs
+ if (/^https?:\/\/(127\.|10\.|192\.168\.|localhost)/i.test(loc)) {
+ return reject(new Error('Redirect to internal address blocked'));
+ }
+ downloadUrlToTempFile(loc, redirectCount + 1).then(resolve).catch(reject);
return;
}
// ... rest unchanged
});
+ req.on('timeout', () => {
+ req.destroy();
+ reject(new Error(`Request timeout after ${TIMEOUT_MS}ms`));
+ });
req.on('error', reject);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bridge/bridge.mjs` around lines 382 - 409, downloadUrlToTempFile currently
follows redirects recursively without limits, lacks request timeouts, and
accepts redirects that can enable SSRF; update downloadUrlToTempFile to accept
an optional redirectCount (default 0) and enforce a maxRedirects constant (e.g.,
5) so any redirect increments redirectCount and rejects if exceeded; before
following a redirect (the loc from res.headers.location) parse it with new URL()
and validate the hostname/ip against a safe allowlist or reject private/reserved
IP ranges to mitigate SSRF; add a request timeout using AbortController or
req.setTimeout and ensure the request is aborted and promise rejected on
timeout; keep existing behavior for writing to tmp and propagate errors
correctly.
Comment on lines
+23
to
+24
| const BRIDGE_PATH = path.resolve(import.meta.dirname, 'bridge.mjs'); | ||
| const WORK_DIR = path.resolve(import.meta.dirname); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
import.meta.dirname Node.js version introduced
💡 Result:
import.meta.dirname was added to Node.js in v20.11.0 (and also in the v21.x line starting at v21.2.0). [1][2]
Docs note: it’s only available for file: modules (not e.g. HTTP/data URLs). [1]
Sources:
[1] Node.js ESM docs (v22.11.0): import.meta.dirname “Added in: v21.2.0, v20.11.0”
[2] Node.js package-examples guide: minimum versions table for import.meta.dirname (v20.11.0 / v21.2.0)
🏁 Script executed:
find . -maxdepth 2 -name "package.json" -type f | head -5Repository: ourines/codes
Length of output: 78
🏁 Script executed:
cat ./bridge/package.jsonRepository: ourines/codes
Length of output: 442
import.meta.dirname requires Node.js 20.11+, but package.json specifies >=18.
import.meta.dirname was introduced in Node.js 20.11.0. Users running Node 18 or 19 will encounter a runtime error when executing this file.
🔧 Proposed fix for Node 18 compatibility
+import { fileURLToPath } from 'node:url';
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+
const HOME = os.homedir();
const NODE_PATH = process.execPath; // e.g. /opt/homebrew/bin/node
-const BRIDGE_PATH = path.resolve(import.meta.dirname, 'bridge.mjs');
-const WORK_DIR = path.resolve(import.meta.dirname);
+const BRIDGE_PATH = path.resolve(__dirname, 'bridge.mjs');
+const WORK_DIR = path.resolve(__dirname);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const BRIDGE_PATH = path.resolve(import.meta.dirname, 'bridge.mjs'); | |
| const WORK_DIR = path.resolve(import.meta.dirname); | |
| import { fileURLToPath } from 'node:url'; | |
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | |
| const HOME = os.homedir(); | |
| const NODE_PATH = process.execPath; // e.g. /opt/homebrew/bin/node | |
| const BRIDGE_PATH = path.resolve(__dirname, 'bridge.mjs'); | |
| const WORK_DIR = path.resolve(__dirname); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bridge/setup-service.mjs` around lines 23 - 24, The code uses
import.meta.dirname (in BRIDGE_PATH and WORK_DIR) which requires Node 20.11+, so
replace that usage with the Node 18-compatible pattern: compute the current
module file path from import.meta.url (via fileURLToPath) and then use
path.dirname to derive the directory, and use that directory to build
BRIDGE_PATH and WORK_DIR; update any imports to ensure fileURLToPath is imported
from 'url' and remove import.meta.dirname references in BRIDGE_PATH and
WORK_DIR.
Comment on lines
+28
to
+68
| const plist = `<?xml version="1.0" encoding="UTF-8"?> | ||
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
| <plist version="1.0"> | ||
| <dict> | ||
| <key>Label</key> | ||
| <string>${LABEL}</string> | ||
|
|
||
| <key>ProgramArguments</key> | ||
| <array> | ||
| <string>${NODE_PATH}</string> | ||
| <string>${BRIDGE_PATH}</string> | ||
| </array> | ||
|
|
||
| <key>WorkingDirectory</key> | ||
| <string>${WORK_DIR}</string> | ||
|
|
||
| <key>RunAtLoad</key> | ||
| <true/> | ||
|
|
||
| <key>KeepAlive</key> | ||
| <true/> | ||
|
|
||
| <key>EnvironmentVariables</key> | ||
| <dict> | ||
| <key>HOME</key> | ||
| <string>${HOME}</string> | ||
| <key>PATH</key> | ||
| <string>/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin</string> | ||
| <key>FEISHU_APP_ID</key> | ||
| <string>${APP_ID}</string> | ||
| <key>FEISHU_APP_SECRET_PATH</key> | ||
| <string>${SECRET_PATH}</string> | ||
| </dict> | ||
|
|
||
| <key>StandardOutPath</key> | ||
| <string>${HOME}/.codes/logs/feishu-bridge.out.log</string> | ||
| <key>StandardErrorPath</key> | ||
| <string>${HOME}/.codes/logs/feishu-bridge.err.log</string> | ||
| </dict> | ||
| </plist> | ||
| `; |
There was a problem hiding this comment.
Potential XML injection in plist template.
If APP_ID, SECRET_PATH, or HOME contain XML special characters (<, >, &, "), the generated plist will be malformed or potentially dangerous. While unlikely in practice, consider escaping these values.
🛡️ Proposed fix to escape XML special characters
+function escapeXml(s) {
+ return String(s)
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"');
+}
+
const plist = `<?xml version="1.0" encoding="UTF-8"?>
...
<key>FEISHU_APP_ID</key>
- <string>${APP_ID}</string>
+ <string>${escapeXml(APP_ID)}</string>
<key>FEISHU_APP_SECRET_PATH</key>
- <string>${SECRET_PATH}</string>
+ <string>${escapeXml(SECRET_PATH)}</string>
...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const plist = `<?xml version="1.0" encoding="UTF-8"?> | |
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | |
| <plist version="1.0"> | |
| <dict> | |
| <key>Label</key> | |
| <string>${LABEL}</string> | |
| <key>ProgramArguments</key> | |
| <array> | |
| <string>${NODE_PATH}</string> | |
| <string>${BRIDGE_PATH}</string> | |
| </array> | |
| <key>WorkingDirectory</key> | |
| <string>${WORK_DIR}</string> | |
| <key>RunAtLoad</key> | |
| <true/> | |
| <key>KeepAlive</key> | |
| <true/> | |
| <key>EnvironmentVariables</key> | |
| <dict> | |
| <key>HOME</key> | |
| <string>${HOME}</string> | |
| <key>PATH</key> | |
| <string>/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin</string> | |
| <key>FEISHU_APP_ID</key> | |
| <string>${APP_ID}</string> | |
| <key>FEISHU_APP_SECRET_PATH</key> | |
| <string>${SECRET_PATH}</string> | |
| </dict> | |
| <key>StandardOutPath</key> | |
| <string>${HOME}/.codes/logs/feishu-bridge.out.log</string> | |
| <key>StandardErrorPath</key> | |
| <string>${HOME}/.codes/logs/feishu-bridge.err.log</string> | |
| </dict> | |
| </plist> | |
| `; | |
| function escapeXml(s) { | |
| return String(s) | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"'); | |
| } | |
| const plist = `<?xml version="1.0" encoding="UTF-8"?> | |
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | |
| <plist version="1.0"> | |
| <dict> | |
| <key>Label</key> | |
| <string>${LABEL}</string> | |
| <key>ProgramArguments</key> | |
| <array> | |
| <string>${NODE_PATH}</string> | |
| <string>${BRIDGE_PATH}</string> | |
| </array> | |
| <key>WorkingDirectory</key> | |
| <string>${WORK_DIR}</string> | |
| <key>RunAtLoad</key> | |
| <true/> | |
| <key>KeepAlive</key> | |
| <true/> | |
| <key>EnvironmentVariables</key> | |
| <dict> | |
| <key>HOME</key> | |
| <string>${HOME}</string> | |
| <key>PATH</key> | |
| <string>/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin</string> | |
| <key>FEISHU_APP_ID</key> | |
| <string>${escapeXml(APP_ID)}</string> | |
| <key>FEISHU_APP_SECRET_PATH</key> | |
| <string>${escapeXml(SECRET_PATH)}</string> | |
| </dict> | |
| <key>StandardOutPath</key> | |
| <string>${HOME}/.codes/logs/feishu-bridge.out.log</string> | |
| <key>StandardErrorPath</key> | |
| <string>${HOME}/.codes/logs/feishu-bridge.err.log</string> | |
| </dict> | |
| </plist> | |
| `; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bridge/setup-service.mjs` around lines 28 - 68, The plist string interpolates
user-controlled values (e.g., APP_ID, SECRET_PATH, HOME and any other ${...}
like LABEL, NODE_PATH, BRIDGE_PATH, WORK_DIR) directly which can break XML;
create and call a small XML-escaping helper (e.g., escapeXml) to replace &, <,
>, " and ' in those variables and use the escaped results when building the
plist template so the generated plist remains well-formed and safe.
Comment on lines
+482
to
+493
| ## 安全注意事项 | ||
|
|
||
| 1. `~/.codes/config.json` 包含 API Key，已设置 600 权限，勿公开 | ||
| 2. `bridge/.env` 包含 HTTP Token，已设置 600 权限 | ||
| 3. codes serve 默认监听 `127.0.0.1:3456`（仅本地），bridge 通过本地回环访问，外部无法直接连接 | ||
| 4. 防火墙只需开放 SSH： | ||
| ```bash | ||
| sudo ufw allow ssh | ||
| sudo ufw enable | ||
| ``` | ||
| 不需要开放 80/443/3456 等端口 — bridge 使用**出站** WebSocket 连接飞书云，无需入站端口 | ||
| 4. 所有 `/assistant` 请求都需要 Bearer Token 认证 |
There was a problem hiding this comment.
Duplicate numbering in security notes list.
There are two items numbered "4." in the security notes section.
📝 Proposed fix
3. codes serve 默认监听 `127.0.0.1:3456`（仅本地），bridge 通过本地回环访问，外部无法直接连接
4. 防火墙只需开放 SSH：
```bash
sudo ufw allow ssh
sudo ufw enable
```
不需要开放 80/443/3456 等端口 — bridge 使用**出站** WebSocket 连接飞书云，无需入站端口
-4. 所有 `/assistant` 请求都需要 Bearer Token 认证
+5. 所有 `/assistant` 请求都需要 Bearer Token 认证📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## 安全注意事项 | |
| 1. `~/.codes/config.json` 包含 API Key，已设置 600 权限，勿公开 | |
| 2. `bridge/.env` 包含 HTTP Token，已设置 600 权限 | |
| 3. codes serve 默认监听 `127.0.0.1:3456`（仅本地），bridge 通过本地回环访问，外部无法直接连接 | |
| 4. 防火墙只需开放 SSH： | |
| ```bash | |
| sudo ufw allow ssh | |
| sudo ufw enable | |
| ``` | |
| 不需要开放 80/443/3456 等端口 — bridge 使用**出站** WebSocket 连接飞书云，无需入站端口 | |
| 4. 所有 `/assistant` 请求都需要 Bearer Token 认证 | |
| ## 安全注意事项 | |
| 1. `~/.codes/config.json` 包含 API Key，已设置 600 权限，勿公开 | |
| 2. `bridge/.env` 包含 HTTP Token，已设置 600 权限 | |
| 3. codes serve 默认监听 `127.0.0.1:3456`（仅本地），bridge 通过本地回环访问，外部无法直接连接 | |
| 4. 防火墙只需开放 SSH： |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@DEPLOY.md` around lines 482 - 493, In the "安全注意事项" list within DEPLOY.md
there are two items labeled "4."; update the second duplicated "4." (the line
starting "所有 `/assistant` 请求都需要 Bearer Token 认证") to "5." so the ordered list
numbers are sequential; locate the list under the "安全注意事项" heading and change
the numbering for that final item from 4. to 5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
从此可以使用飞书愉快的操作 codes 了……
飞书方案来自 https://github.com/AlexAnys/feishu-openclaw
Summary by CodeRabbit
New Features
Documentation