Sync/upstream 2026 03 08#99
Conversation
…@joetomasone) Land openclaw#9040 by @joetomasone. Add fail-closed config loading, compat coverage, and changelog entry for openclaw#5052. Co-authored-by: Joe Tomasone <joe@tomasone.com>
… thanks @riftzen-bit) Co-authored-by: riftzen-bit <binb53339@gmail.com>
…lsdcc01) Land openclaw#33757 by @lsdcc01 without the unrelated dependency bump. Preserve negative FTS5 BM25 ordering in hybrid scoring and add changelog coverage for openclaw#5767. Co-authored-by: 丁春才0668000523 <ding.chuncai1@xydigit.com>
…claw#39230, thanks @ademczuk) Co-authored-by: ademczuk <andrew.demczuk@gmail.com>
…, thanks @ademczuk) Co-authored-by: ademczuk <andrew.demczuk@gmail.com>
…anks @ademczuk) Co-authored-by: ademczuk <andrew.demczuk@gmail.com>
Landed from contributor PR openclaw#28428 by @l0cka. Co-authored-by: Daniel Alkurdi <danielalkurdi@gmail.com>
… thanks @Gundam98) Co-authored-by: Gundam98 <huhanwen98@gmail.com>
Landed from contributor PR openclaw#28608 by @KimGLee. Co-authored-by: Kim <150593189+KimGLee@users.noreply.github.com>
Co-authored-by: Tom <hxtxmu@gmail.com>
macOS app/chat/browser/cron/permissions fixes. Co-authored-by: ImHermes1 <lukeforn@gmail.com>
Resolve merge conflicts across agent tools, daemon services, and tests while preserving QVerisBot fork integrations. Restore daemon service install wrapper compatibility and verify with full pnpm test. Made-with: Cursor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the build and deployment processes across Docker, iOS, and macOS platforms, while simultaneously bolstering the robustness and security of various communication channel integrations. It introduces new functionalities such as browser control for macOS nodes and advanced MCP server configuration for ACPX, alongside significant refinements to the chat user interface and extensive documentation updates. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a wide range of improvements across the repository, including a major refactoring of the Android app's package structure, significant enhancements to the Dockerfile for smaller and more secure images, and various UI/UX and security hardening updates for the iOS and macOS applications. The Fastlane configuration for iOS deployment has also been substantially improved. However, critical security vulnerabilities were identified in the new acpx extension and the macOS browser proxy logic. Specifically, the acpx extension is vulnerable to arbitrary command execution via untrusted agent names and leaks secrets through process command-line arguments, and the macOS browser proxy introduces a risk of arbitrary file read if the local browser control service is manipulated. Additionally, a configuration issue with detect-secrets was found, involving redundant and partially incorrect filter patterns in the .pre-commit-config.yaml file. These security vulnerabilities and configuration issues should be addressed before merging.
| cwd: params.cwd, | ||
| spawnOptions: params.spawnOptions, | ||
| }); | ||
| return overrides[normalizedAgent] ?? ACPX_BUILTIN_AGENT_COMMANDS[normalizedAgent] ?? params.agent; |
There was a problem hiding this comment.
The acpx extension's resolveAcpxAgentCommand function falls back to using the provided agent name as a command if it is not found in the built-in commands or configured overrides. This agent name is then passed to mcp-proxy.mjs, which splits it into a command and arguments and executes it using child_process.spawn. Since the agent name can be influenced by Large Language Model (LLM) tool calls or remote gateway requests, an attacker can execute arbitrary binaries on the host system.
| }), | ||
| "utf8", | ||
| ).toString("base64url"); | ||
| return toCommandLine([process.execPath, MCP_PROXY_PATH, "--payload", payload]); |
There was a problem hiding this comment.
The acpx extension passes a base64url-encoded JSON payload to the mcp-proxy.mjs process as a command-line argument. This payload contains the configuration for MCP servers, including environment variables that often hold sensitive information such as API keys. Command-line arguments are visible to all users on the system (e.g., via the ps command), leading to the exposure of these secrets.
| let url = URL(fileURLWithPath: path) | ||
| let values = try url.resourceValues(forKeys: [.isRegularFileKey, .fileSizeKey]) | ||
| guard values.isRegularFile == true else { | ||
| throw NSError(domain: "MacNodeBrowserProxy", code: 7, userInfo: [ | ||
| NSLocalizedDescriptionKey: "browser proxy file not found: \(path)", | ||
| ]) | ||
| } | ||
| if let fileSize = values.fileSize, fileSize > Self.maxProxyFileBytes { | ||
| throw NSError(domain: "MacNodeBrowserProxy", code: 8, userInfo: [ | ||
| NSLocalizedDescriptionKey: "browser proxy file exceeds 10MB: \(path)", | ||
| ]) | ||
| } | ||
|
|
||
| let data = try Data(contentsOf: url) | ||
| let mimeType = UTType(filenameExtension: url.pathExtension)?.preferredMIMEType | ||
| return ProxyFilePayload(path: path, base64: data.base64EncodedString(), mimeType: mimeType) | ||
| } |
There was a problem hiding this comment.
The MacNodeBrowserProxy actor reads files from the local disk based on paths returned in the JSON response from a local browser control service. An attacker (e.g., a compromised or malicious gateway) can use the browser.proxy command to send arbitrary HTTP requests to the local service. If the local service can be tricked into returning a path to a sensitive file (e.g., via an echo endpoint or a download feature), the MacNodeBrowserProxy will read that file and return its content to the gateway. It is recommended to restrict file reading to a specific, allow-listed directory and validate that any path returned by the local service is within this allowed directory.
| - --exclude-lines | ||
| - '"secretShape": "(secret_input|sibling_ref)"' | ||
| - --exclude-lines | ||
| - 'API key rotation \(provider-specific\): set `\*_API_KEYS`' | ||
| - --exclude-lines | ||
| - 'password: `OPENCLAW_GATEWAY_PASSWORD` -> `gateway\.auth\.password` -> `gateway\.remote\.password`' | ||
| - --exclude-lines | ||
| - 'password: `OPENCLAW_GATEWAY_PASSWORD` -> `gateway\.remote\.password` -> `gateway\.auth\.password`' | ||
| - --exclude-files | ||
| - '^src/gateway/client\.watchdog\.test\.ts$' | ||
| - --exclude-lines | ||
| - 'export CUSTOM_API_K[E]Y="your-key"' | ||
| - --exclude-lines | ||
| - 'grep -q ''N[O]DE_COMPILE_CACHE=/var/tmp/openclaw-compile-cache'' ~/.bashrc \|\| cat >> ~/.bashrc <<''EOF''' | ||
| - --exclude-lines | ||
| - 'env: \{ MISTRAL_API_K[E]Y: "sk-\.\.\." \},' | ||
| - --exclude-lines | ||
| - '"ap[i]Key": "xxxxx",' | ||
| - --exclude-lines | ||
| - 'ap[i]Key: "A[I]za\.\.\.",' |
There was a problem hiding this comment.
These --exclude-lines appear to be redundant with the patterns defined in .detect-secrets.cfg. The detect-secrets tool automatically discovers and uses the .detect-secrets.cfg file, which makes these command-line arguments unnecessary and complicates configuration management.
Furthermore, some of the regular expressions are incorrect. For instance, on lines 57 and 59, the dots in gateway.auth.password are not escaped, causing them to act as wildcards. The passw[o]rd pattern, used in .detect-secrets.cfg to prevent self-detection, is also missing here.
To simplify the configuration and fix these issues, I recommend removing these redundant --exclude-lines and relying solely on .detect-secrets.cfg for all filter definitions.
No description provided.