Skip to content

fix: Resolve ClawHub suspicious flag by making service opt-in#20

Open
dreamwing wants to merge 1 commit intomasterfrom
fix-suspicious-flag
Open

fix: Resolve ClawHub suspicious flag by making service opt-in#20
dreamwing wants to merge 1 commit intomasterfrom
fix-suspicious-flag

Conversation

@dreamwing
Copy link
Copy Markdown
Owner

Description

Resolves the 'Suspicious' flag on ClawHub by stripping high-risk behaviors from the default installation path.

Changes

  1. SKILL.md update: Removed curl | bash from <install> tag, replacing it with a safe git clone path.
  2. Opt-in Service: setup.sh no longer automatically registers a systemd/launchd service. This requires an --enable-service flag.
  3. No Outer Binaries: Stopped the automatic background download of cloudflared. It's now BYOB (Bring Your Own Binary) with helpful prompt overrides.
  4. Preserved UX: install.sh passes the --enable-service flag inherently, keeping the one-click experience intact for human users.
  5. Hardened Credentials: Added chmod 600 .env.

…ipping remote payloads

Changes include:

- : Removed curl | bash from <install> tag, replaced with safe git clone.

- : Made systemd/launchd service registration strictly opt-in via `--enable-service`.

- : Discontinued automatic download of cloudflared binary (BYOB).

- : Prevented automatic binary download, ensuring users install it manually.

- : Preserved one-click installer behavior by silently passing `--enable-service`.

- : Added `chmod 600` to `.env` file for credential security.
@dreamwing
Copy link
Copy Markdown
Owner Author

@codex Review pls

@dreamwing
Copy link
Copy Markdown
Owner Author

@greptileai Review pls

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb545b2011

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +158 to +159
if [ "$ENABLE_SERVICE" = true ]; then
if [ "$OS_TYPE" = "Darwin" ]; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Skip service restart when service mode is disabled

This new opt-in gate allows setup.sh to run without registering launchd/systemd, but the tunnel section still unconditionally restarts a service later (launchctl load / systemctl restart). In the default --enable-service-absent flow, users who enable tunnel setup hit those restart commands with no unit/plist present, and because the script runs with set -e, setup aborts instead of completing. Guard the restart block behind ENABLE_SERVICE=true (or start the app directly in non-service mode).

Useful? React with 👍 / 👎.

<!-- Installation -->
<install>
curl -sL https://raw.githubusercontent.com/dreamwing/clawbridge/master/install.sh | bash
git clone https://github.com/dreamwing/clawbridge.git skills/clawbridge && cd skills/clawbridge && npm install --production && node index.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Run setup before launching app in skill install command

The new <install> command starts node index.js directly, but fresh installs do not have .env yet and src/config.js exits when ACCESS_KEY is unset. That means the documented install path fails to boot and does not generate credentials for login. Call ./setup.sh (or otherwise create .env with ACCESS_KEY) before starting the Node process.

Useful? React with 👍 / 👎.

Comment on lines +344 to 345
ENABLE_TUNNEL="n"
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Disable tunnel env writes when cloudflared is missing

When cloudflared is not installed, this branch sets ENABLE_TUNNEL="n", but a supplied --token is still preserved and later written back with ENABLE_EMBEDDED_TUNNEL=true. In that case every app start still attempts embedded tunnel startup and fails in tunnel.downloadBinary(), so remote access remains broken until users manually clean .env. Clear CF_TOKEN or skip tunnel env writes after the missing-binary check.

Useful? React with 👍 / 👎.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 14, 2026

Greptile Summary

This PR refactors ClawBridge to remove high-risk default behaviours that were triggering ClawHub's "Suspicious" flag: service registration is now opt-in via --enable-service, automatic cloudflared binary downloads are replaced with BYOB guidance, and the default <install> path swaps curl | bash for git clone. The intent and structure of the changes are sound, but a logic regression and several stale documentation entries were introduced.

Key findings:

  • Logic regression in setup.sh — The service-restart block inside the Cloudflare tunnel section (lines 391-403) runs unconditionally regardless of ENABLE_SERVICE. When --enable-service was not passed, launchctl load (macOS) and systemctl restart (Linux) target units that were never created, causing hard failures under set -e. This will crash setup.sh for any user who enables a tunnel without also enabling the service.
  • <install> in SKILL.md skips setup.sh — The new install command ends with node index.js without first running setup.sh. This means no .env is generated, no ACCESS_KEY is created, and users have no credentials to log in with after the clone.
  • Stale metadata in SKILL.md — The <dependency>, <network>, and <filesystem> XML blocks, plus the markdown "What This Skill Does" section (lines 97-98), still describe automatic cloudflared downloads and unconditional service registration — both of which were explicitly removed by this PR.
  • Misleading function name in tunnel.jsdownloadBinary() no longer downloads anything; it resolves an existing binary. The name should be updated to avoid confusion.

Confidence Score: 2/5

  • Not safe to merge — the service restart regression will crash setup.sh for direct users who enable a tunnel without the service flag.
  • The core opt-in service change and BYOB cloudflared approach are correct, but the untouched service-restart block inside the tunnel section introduces a hard crash under set -e that affects a common direct-invocation path. The <install> command also leaves users unable to authenticate after cloning. Both issues need to be resolved before the PR achieves its stated goal of a cleaner, safer install experience.
  • setup.sh (lines 391-403) requires the most attention — the restart block must be gated on the service being enabled. SKILL.md also needs its <install> command and several stale metadata sections updated.

Important Files Changed

Filename Overview
setup.sh Adds --enable-service opt-in flag correctly, but the service-restart block inside the Cloudflare tunnel section was not gated on the same flag, causing a hard set -e failure when the service was not registered.
SKILL.md The <install> command now uses git clone but runs node index.js without setup.sh, skipping .env generation and leaving users without an ACCESS_KEY. Multiple metadata sections (<dependency>, <network>, <filesystem>, and the markdown description) still describe the old automatic cloudflared-download and unconditional service-registration behaviour.
tunnel.js Binary-download logic correctly replaced with a local-path check plus which cloudflared fallback; function name downloadBinary is now misleading but otherwise the change is clean.
install.sh Correctly passes --enable-service to both setup.sh invocation paths, preserving the one-click UX while making service registration opt-in for direct setup.sh users.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([install.sh]) --> B[Clone + npm install]
    B --> C{Permanent tunnel\nalready in .env?}
    C -- yes --> D[setup.sh with token\nand enable-service]
    C -- no --> E[setup.sh quick\nand enable-service]

    G([Direct: setup.sh]) --> H{enable-service\nflag passed?}
    D --> H
    E --> H

    H -- yes --> I[Write systemd / launchd\nservice unit file]
    H -- no --> J[Skip service registration]

    I --> K[Start service]
    K --> L{Cloudflare tunnel\nrequested?}
    J --> L

    L -- no --> S[Print summary]
    L -- yes --> M{cloudflared found\nin PATH or locally?}

    M -- yes --> N[Write tunnel config to .env]
    M -- no --> O[Print BYOB install instructions\nset ENABLE_TUNNEL to n]

    N --> P{ENABLE_SERVICE\nwas true?}
    O --> BUG["⚠️ BUG: restart block runs anyway\nlaunchctl load or systemctl restart\non missing unit — set -e terminates script"]
    P -- yes --> R[Restart service]
    P -- no --> BUG

    R --> S
Loading

Comments Outside Diff (2)

  1. SKILL.md, line 23 (link)

    Several SKILL.md sections still describe the old automatic-download behaviour

    Four separate locations in this file were not updated to reflect the BYOB (--enable-service opt-in) changes introduced in this PR:

    1. Line 23 — <dependency> description for cloudflared: still says "Downloaded automatically from github.com/cloudflare/cloudflared if Cloudflare tunnel is enabled." — this is no longer true.
    2. Line 39 — <network> entry: trigger still reads "install (only if tunnel enabled and cloudflared not found)", implying the binary is fetched automatically.
    3. Line 48 — <filesystem> entry: the systemd path is described unconditionally, but it's now only written when --enable-service is passed.
    4. Lines 97–98 — Markdown section:
      • "Registers a user-level systemd service for auto-start" — now opt-in.
      • "Optionally downloads cloudflared" — no longer downloaded at all.

    These stale entries will confuse both human readers and AI agents that parse the <skill> metadata to decide whether to install the skill.

  2. setup.sh, line 391-403 (link)

    Service restart block runs even without --enable-service

    This restart block lives inside the Cloudflare tunnel section and runs unconditionally, regardless of whether the user passed --enable-service. When the flag was not provided, no service unit file was created, so both restart paths fail under set -e:

    • macOS: launchctl load -w targets a .plist path that was never written. There is no || true guard on that line, so the script terminates here.
    • Linux: USE_USER_SYSTEMD is only assigned inside the if ENABLE_SERVICE block. Without the flag it is unset, the elif evaluates false, and the else branch runs sudo systemctl restart against a unit that was never installed — another hard failure.

    Any user invoking ./setup.sh --quick or ./setup.sh --token directly (without --enable-service) who also enables a Cloudflare tunnel will hit this crash and never see the §6 summary output.

    The fix is to wrap this entire restart block in a guard that only runs when the service was actually registered.

Last reviewed commit: eb545b2

<!-- Installation -->
<install>
curl -sL https://raw.githubusercontent.com/dreamwing/clawbridge/master/install.sh | bash
git clone https://github.com/dreamwing/clawbridge.git skills/clawbridge && cd skills/clawbridge && npm install --production && node index.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

<install> skips setup.sh, so no ACCESS_KEY is generated

The new <install> command runs node index.js directly:

git clone https://github.com/dreamwing/clawbridge.git skills/clawbridge && cd skills/clawbridge && npm install --production && node index.js

setup.sh is what generates the .env file with a random ACCESS_KEY (and PORT). Without running it first, the app starts with no credentials — users have no access key to log in with, and no port override if 3000 is busy. The <install> path should run setup.sh before starting the process:

Suggested change
git clone https://github.com/dreamwing/clawbridge.git skills/clawbridge && cd skills/clawbridge && npm install --production && node index.js
git clone https://github.com/dreamwing/clawbridge.git skills/clawbridge && cd skills/clawbridge && npm install --production && ./setup.sh && node index.js

Comment on lines 10 to 26
async function downloadBinary() {
// Check local binary first (legacy or manual placement)
if (fs.existsSync(BIN_PATH) && fs.statSync(BIN_PATH).size > 1000000) return;

const url = getDownloadUrl();
console.log(`[Tunnel] Downloading cloudflared for ${os.platform()}/${os.arch()}...`);

const https = require('https');
const http = require('http');
// Check system PATH
const { execSync } = require('child_process');
try {
const systemPath = execSync('which cloudflared', { encoding: 'utf8' }).trim();
if (systemPath && fs.existsSync(systemPath)) {
BIN_PATH = systemPath;
return;
}
} catch (e) { /* expected if not found in PATH */ }

return new Promise((resolve, reject) => {
const download = (downloadUrl, redirects = 0) => {
if (redirects > 5) return reject(new Error('Too many redirects'));
const client = downloadUrl.startsWith('https') ? https : http;
client.get(downloadUrl, (res) => {
// Follow redirects (GitHub releases use 302)
if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
return download(res.headers.location, redirects + 1);
}
if (res.statusCode !== 200) {
return reject(new Error(`Download failed: HTTP ${res.statusCode}`));
}
const file = fs.createWriteStream(BIN_PATH);
res.pipe(file);
file.on('finish', () => {
file.close();
fs.chmodSync(BIN_PATH, '755');
console.log('[Tunnel] Download complete.');
resolve();
});
file.on('error', (err) => {
fs.unlinkSync(BIN_PATH);
reject(err);
});
}).on('error', reject);
};
download(url);
});
// If both fail, throw error so index.js catches it and doesn't start tunnel
throw new Error('cloudflared not found. Install via: brew install cloudflared (macOS) or apt install cloudflared (Linux)');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

downloadBinary is a misleading name — it no longer downloads anything

The function was renamed in spirit but not in name: it now only resolves an existing binary (checks local path, then falls back to which cloudflared). Calling it downloadBinary will confuse future maintainers who expect it to fetch a remote binary, and may cause callers in index.js to mis-handle the thrown error (expecting a network failure rather than a "binary not found" signal).

Consider renaming it to resolveBinary or findCloudflared to match its new responsibility.

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