mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
chore(autoreview): centralize secret scanning (#112036)
* chore(autoreview): centralize secret scanning * chore(autoreview): remove generated caches * chore(autoreview): remove generated caches
This commit is contained in:
@@ -17,6 +17,8 @@ Use when:
|
||||
- after non-trivial code edits, before final/commit/ship
|
||||
- reviewing a local branch or PR branch after fixes
|
||||
|
||||
Do not require autoreview for a change whose entire diff is prose-only internal notes or `SKILL.md` documentation. Still inspect the diff directly and run the repository's lightweight documentation validation, if any. This exception does not cover user-facing documentation, executable examples, configuration, scripts, generated files, or behavior changes.
|
||||
|
||||
## Contract
|
||||
|
||||
- Treat review output as advisory. Never blindly apply it.
|
||||
@@ -36,7 +38,7 @@ Use when:
|
||||
- Tools are useful in review mode. Codex receives the validated bundle in an empty workspace so ignored files and linked-worktree metadata remain unreadable; web search stays available for dependency contracts and upstream docs.
|
||||
- Security perspective is always included, but it should not cripple legitimate functionality. Report security findings only when the change creates a concrete, actionable risk or removes an important safety check.
|
||||
- Reviewer subprocesses preserve engine authentication and non-credentialed proxy variables needed by headless or restricted-network environments while stripping process-injection, Git override, and credentialed proxy values.
|
||||
- Review bundles fail closed before engine invocation when tracked or untracked paths look sensitive or patch text looks secret-like. Obvious synthetic values shaped like `<fixture-prefix>-<credential-field>` remain reviewable, such as `token: "test-token"`, without one-off allowlists. Safe large diffs are scanned in full, sent as one pass while they fit the aggregate prompt limit, then partitioned into complete bounded passes without truncation.
|
||||
- Before engine invocation, autoreview runs TruffleHog over temporary snapshots of the exact added or modified content under review. It intentionally matches TruffleHog's low-false-positive pre-commit policy (`verified,unknown`); it does not classify arbitrary password-like strings or rescan unchanged history. Install TruffleHog using its official platform-neutral instructions; autoreview fails with that link when the binary is unavailable and never auto-installs it. Repositories should also run TruffleHog in pull-request CI as a backup outside autoreview; repository-local Git hooks are optional. Review bundles still omit security-sensitive paths or files, and explicit prompt and dataset inputs remain checked before engine invocation. Safe large diffs are sent as one pass while they fit the aggregate prompt limit, then partitioned into complete bounded passes without truncation.
|
||||
- For regression provenance, keep roles separate: blamed code author, blamed PR author, PR merger/committer, current PR author, and PR/date. If no blamed PR is traceable, use the blamed commit as the provenance: commit SHA, date, and author username. Do not guess a merger or frame missing PR metadata as a separate finding.
|
||||
- If the blamed PR was merged by `clawsweeper[bot]` or another automation, identify the human trigger when practical. Check timeline/comments first; if rate-limited, use gitcrawl/cache or public PR HTML. Look for maintainer commands such as `@clawsweeper automerge`, `/landpr`, or labels/status comments that armed automerge. Report `automerge triggered by @login`; if not found, say trigger unknown.
|
||||
- Do not invoke built-in `codex review`, nested reviewers, or reviewer panels from inside the review. The helper builds one validated bundle, calls the selected engine once for normal inputs or once per complete bounded chunk for oversized inputs, validates the structured results, and stops.
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -102,6 +102,80 @@ class AutoreviewCursorTests(unittest.TestCase):
|
||||
self.assertIn("review engine result was not structured JSON", str(exc_info.exception))
|
||||
|
||||
|
||||
class AutoreviewSecretScannerTests(unittest.TestCase):
|
||||
def test_boolean_declarations_are_not_credential_material(self) -> None:
|
||||
secret_field = "is" + "Secret"
|
||||
client_secret_field = "hasClient" + "Secret"
|
||||
cases = (
|
||||
(f"val {secret_field}: Boolean? = null,", None),
|
||||
(f"var {client_secret_field}: Boolean = false", None),
|
||||
(f"abstract val {secret_field}: Boolean?", None),
|
||||
(f"val {secret_field}: Boolean?", None),
|
||||
(f"const {client_secret_field}: boolean = true;", "typescript"),
|
||||
(f"declare const {client_secret_field}: boolean;", "typescript"),
|
||||
(f"let {secret_field}: Bool? = nil", None),
|
||||
(f"let {secret_field}: Bool?", None),
|
||||
)
|
||||
|
||||
for content, javascript_dialect in cases:
|
||||
with self.subTest(content=content):
|
||||
self.assertFalse(
|
||||
AUTOREVIEW.secret_text_risk(
|
||||
content,
|
||||
javascript_dialect=javascript_dialect,
|
||||
)
|
||||
)
|
||||
|
||||
def test_boolean_and_null_literal_values_are_not_credentials(self) -> None:
|
||||
cases = (
|
||||
("is" + "Secret", "true"),
|
||||
("requires" + "Password", "false"),
|
||||
("access" + "Token", "null"),
|
||||
)
|
||||
for field_name, literal in cases:
|
||||
content = f"{field_name} = {literal}"
|
||||
with self.subTest(content=content):
|
||||
self.assertFalse(AUTOREVIEW.secret_text_risk(content))
|
||||
|
||||
def test_boolean_annotation_does_not_hide_real_credential_literal(self) -> None:
|
||||
literal_value = "actual-production-" + "secret"
|
||||
secret_field = "is" + "Secret"
|
||||
client_secret_field = "hasClient" + "Secret"
|
||||
cases = (
|
||||
(f'val {secret_field}: Boolean? = "{literal_value}",', None),
|
||||
(f'var {client_secret_field}: Boolean = "{literal_value}"', None),
|
||||
(
|
||||
f'const {client_secret_field}: boolean = "{literal_value}";',
|
||||
"typescript",
|
||||
),
|
||||
(f'let {secret_field}: Bool? = "{literal_value}"', None),
|
||||
)
|
||||
|
||||
for content, javascript_dialect in cases:
|
||||
with self.subTest(content=content):
|
||||
self.assertTrue(
|
||||
AUTOREVIEW.secret_text_risk(
|
||||
content,
|
||||
javascript_dialect=javascript_dialect,
|
||||
)
|
||||
)
|
||||
|
||||
def test_boolean_prefix_values_remain_credentials(self) -> None:
|
||||
field_name = "client" + "Secret"
|
||||
for prefix in ("Boolean", "boolean", "Bool"):
|
||||
literal_value = prefix + "-prod-credential"
|
||||
content = f"{field_name}: {literal_value}"
|
||||
with self.subTest(content=content):
|
||||
self.assertTrue(AUTOREVIEW.secret_text_risk(content))
|
||||
|
||||
def test_boolean_type_tokens_in_config_remain_credentials(self) -> None:
|
||||
field_name = "client" + "Secret"
|
||||
for literal_value in ("Boolean?", "Boolean?=abc1234"):
|
||||
content = f"{field_name}: {literal_value}"
|
||||
with self.subTest(content=content):
|
||||
self.assertTrue(AUTOREVIEW.secret_text_risk(content))
|
||||
|
||||
|
||||
class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
@classmethod
|
||||
def setUpClass(cls) -> None:
|
||||
@@ -553,8 +627,13 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
source.write_text("after\n")
|
||||
|
||||
cursor_bin = root / "cursor-agent"
|
||||
trufflehog_bin = root / "trufflehog"
|
||||
record_path = root / "record.json"
|
||||
AUTOREVIEW.write_executable(cursor_bin, AUTOREVIEW.fake_cursor_script())
|
||||
AUTOREVIEW.write_executable(
|
||||
trufflehog_bin,
|
||||
"#!/usr/bin/env python3\nraise SystemExit(0)\n",
|
||||
)
|
||||
env = os.environ.copy()
|
||||
env.update(
|
||||
{
|
||||
@@ -563,7 +642,10 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
"GIT_CONFIG_GLOBAL": str(root / "hostile-gitconfig"),
|
||||
"NODE_OPTIONS": "--require=hostile.js",
|
||||
"PYTHONPATH": str(root / "hostile-python"),
|
||||
"PATH": f"{repo}{os.pathsep}{env.get('PATH', '')}",
|
||||
"PATH": (
|
||||
f"{root}{os.pathsep}{repo}{os.pathsep}"
|
||||
f"{env.get('PATH', '')}"
|
||||
),
|
||||
"HOME": str(root),
|
||||
"USERPROFILE": str(root),
|
||||
}
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
declare const accountId: string;
|
||||
declare const filePath: string;
|
||||
declare const secretRef: string;
|
||||
declare const tryReadSecretFileSync: (...args: unknown[]) => string;
|
||||
declare const normalizeResolvedSecretInputString: (options: unknown) => string;
|
||||
|
||||
export const passwordFile = tryReadSecretFileSync(filePath, "IRC password file", {
|
||||
credentialDiagnostic: {
|
||||
configPath: `channels.irc.accounts.${accountId}.passwordFile`,
|
||||
},
|
||||
});
|
||||
export const nickservFile = tryReadSecretFileSync(filePath, "IRC NickServ password file", {
|
||||
credentialDiagnostic: {
|
||||
configPath: `channels.irc.accounts.${accountId}.nickserv.passwordFile`,
|
||||
},
|
||||
});
|
||||
export const botSecret = normalizeResolvedSecretInputString({
|
||||
value: secretRef,
|
||||
path: `channels.nextcloud-talk.accounts.${accountId}.botSecret`,
|
||||
});
|
||||
export const botSecretFile = tryReadSecretFileSync(filePath, "Nextcloud bot secret file", {
|
||||
credentialDiagnostic: {
|
||||
configPath: `channels.nextcloud-talk.accounts.${accountId}.botSecretFile`,
|
||||
},
|
||||
});
|
||||
export const tokenFile = tryReadSecretFileSync(
|
||||
filePath,
|
||||
`channels.telegram.accounts.${accountId}.tokenFile`,
|
||||
{ rejectSymlink: true },
|
||||
);
|
||||
@@ -0,0 +1,55 @@
|
||||
type SecretRef = { source: "env"; id: string };
|
||||
type CredentialUnavailableDiagnostic = { path: string; reason: string };
|
||||
|
||||
declare const tokenRef: SecretRef;
|
||||
declare const keyRef: SecretRef;
|
||||
declare const inlinePassword: string;
|
||||
declare const inlineSecret: string;
|
||||
declare const accountFileToken: string;
|
||||
declare const baseFileToken: string;
|
||||
declare const passwordResolution: { password: string };
|
||||
declare const secretResolution: { secret: string };
|
||||
declare const tokenResolution: { token: string };
|
||||
declare const accountTokenFile: { token: string };
|
||||
declare const channelTokenFile: { token: string };
|
||||
declare const merged: { apiPassword: string; passwordFile: string };
|
||||
declare const tryReadSecretFileSync: (...args: unknown[]) => string;
|
||||
declare const normalizeResolvedSecretInputString: (options: unknown) => string;
|
||||
declare const resolveToken: (options: unknown) => { value: string };
|
||||
|
||||
const filePassword = tryReadSecretFileSync(merged.passwordFile, "IRC password file", {
|
||||
credentialDiagnostic: {
|
||||
configPath: `channels.irc.accounts.${accountId}.passwordFile`,
|
||||
report: (diagnostic: CredentialUnavailableDiagnostic) => diagnostic,
|
||||
},
|
||||
});
|
||||
const configPassword = normalizeResolvedSecretInputString({
|
||||
value: merged.apiPassword,
|
||||
path: "channels.nextcloud-talk.apiPassword",
|
||||
});
|
||||
const token = resolveToken({ accountId });
|
||||
const priorPasswordFileError = /IRC password file.*must not be a symlink/;
|
||||
|
||||
export type CredentialPlumbing = {
|
||||
tokenRef?: SecretRef;
|
||||
keyRef?: SecretRef;
|
||||
credentialDiagnostics?: CredentialUnavailableDiagnostic[];
|
||||
};
|
||||
|
||||
export const resolvedCredentialPlumbing = {
|
||||
token: tokenRef,
|
||||
apiKey: keyRef,
|
||||
password: filePassword,
|
||||
configPassword,
|
||||
nextPassword: inlinePassword,
|
||||
secret: inlineSecret,
|
||||
accountToken: accountFileToken,
|
||||
baseToken: baseFileToken,
|
||||
resolvedPassword: passwordResolution.password,
|
||||
resolvedSecret: secretResolution.secret,
|
||||
resolvedToken: tokenResolution.token,
|
||||
accountTokenFile: accountTokenFile.token,
|
||||
channelTokenFile: channelTokenFile.token,
|
||||
apiPassword: merged.apiPassword,
|
||||
channelAccessToken: token.value,
|
||||
};
|
||||
@@ -0,0 +1,10 @@
|
||||
const password = "FAKE-CorrectHorseBattery-Staple-2026!";
|
||||
const credential = "FAKE_A7f9K2m4Q8v6N3x5R1p0T9z8";
|
||||
const apiKey = "sk-proj-FAKE00000000000000000000000000000000000000000000";
|
||||
const githubToken = "ghp_FAKE000000000000000000000000000000";
|
||||
const awsAccessKey = "AKIAFAKE000000000000";
|
||||
const slackToken = "xoxb-FAKE000000000-FAKE000000000-FAKE000000000000000000000000";
|
||||
const authorization = "Bearer eyJhbGciOiJIUzI1NiJ9.RkFLRS1OT1QtQS1SRUFM.TOKENFAKESIGNATURE";
|
||||
const resolvedToken = resolveToken({ value: "FAKE_B8g0L3n5R9w7P4y6S2q1U0a9" });
|
||||
const filePassword = tryReadSecretFileSync(path, "FAKE-A7f9K2m4Q8v6N3x5R1p0T9z8");
|
||||
const password = readPassword("alice", "FAKE correct horse secret battery 2026");
|
||||
File diff suppressed because it is too large
Load Diff
@@ -40,17 +40,6 @@ if [ "${#files[@]}" -eq 0 ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if ! command -v trufflehog >/dev/null 2>&1; then
|
||||
cat >&2 <<'EOF'
|
||||
OpenClaw requires TruffleHog for pre-commit secret scanning.
|
||||
|
||||
Install it, then retry the commit:
|
||||
macOS: brew install trufflehog
|
||||
Other platforms: https://github.com/trufflesecurity/trufflehog#installation
|
||||
EOF
|
||||
exit 1
|
||||
fi
|
||||
|
||||
restage_files=()
|
||||
for file in "${files[@]}"; do
|
||||
if ! git check-ignore --no-index -q -- "$file"; then
|
||||
@@ -70,22 +59,3 @@ fi
|
||||
if [ "${#restage_files[@]}" -gt 0 ]; then
|
||||
git add -- "${restage_files[@]}"
|
||||
fi
|
||||
|
||||
staged_snapshot="$(mktemp -d "${TMPDIR:-/tmp}/openclaw-trufflehog.XXXXXX")"
|
||||
trap 'rm -rf "$staged_snapshot"' EXIT
|
||||
for file in "${files[@]}"; do
|
||||
if [[ "$(git cat-file -t ":0:$file")" != "blob" ]]; then
|
||||
continue
|
||||
fi
|
||||
snapshot_path="$staged_snapshot/$file"
|
||||
mkdir -p "${snapshot_path%/*}"
|
||||
git cat-file blob ":0:$file" > "$snapshot_path"
|
||||
done
|
||||
|
||||
trufflehog \
|
||||
--no-update \
|
||||
--no-color \
|
||||
--results=verified,unknown \
|
||||
--fail \
|
||||
--fail-on-scan-errors \
|
||||
filesystem "$staged_snapshot"
|
||||
|
||||
@@ -80,7 +80,6 @@ function installPreCommitFixture(dir: string): string {
|
||||
const fakeBinDir = path.join(dir, "bin");
|
||||
mkdirSync(fakeBinDir, { recursive: true });
|
||||
writeExecutable(fakeBinDir, "node", "#!/usr/bin/env bash\nexit 0\n");
|
||||
writeExecutable(fakeBinDir, "trufflehog", "#!/usr/bin/env bash\nexit 0\n");
|
||||
return fakeBinDir;
|
||||
}
|
||||
|
||||
@@ -261,95 +260,6 @@ describe("git-hooks/pre-commit (integration)", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("scans only staged versions of changed files with TruffleHog", () => {
|
||||
const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-trufflehog-");
|
||||
run(dir, "git", ["init", "-q", "--initial-branch=main"]);
|
||||
const fakeBinDir = installPreCommitFixture(dir);
|
||||
const logPath = path.join(dir, "trufflehog.log");
|
||||
writeExecutable(
|
||||
fakeBinDir,
|
||||
"trufflehog",
|
||||
`#!/usr/bin/env bash
|
||||
printf 'env=%s\n' "\${TRUFFLEHOG_PRE_COMMIT:-}" > ${JSON.stringify(logPath)}
|
||||
printf 'args=%s\n' "$*" >> ${JSON.stringify(logPath)}
|
||||
`,
|
||||
);
|
||||
|
||||
writeFileSync(path.join(dir, "base.txt"), "base\n", "utf8");
|
||||
run(dir, "git", ["add", "--", "base.txt"]);
|
||||
run(dir, "git", [
|
||||
"-c",
|
||||
"user.name=Test User",
|
||||
"-c",
|
||||
"user.email=test@example.invalid",
|
||||
"commit",
|
||||
"-q",
|
||||
"-m",
|
||||
"base",
|
||||
]);
|
||||
writeFileSync(path.join(dir, "changed.txt"), "safe staged content\n", "utf8");
|
||||
run(dir, "git", ["add", "--", "changed.txt"]);
|
||||
|
||||
run(dir, "bash", ["git-hooks/pre-commit"], {
|
||||
PATH: `${fakeBinDir}:${process.env.PATH ?? ""}`,
|
||||
});
|
||||
|
||||
const log = readFileSync(logPath, "utf8");
|
||||
expect(log).toContain("env=\n");
|
||||
expect(log).toContain(
|
||||
"args=--no-update --no-color --results=verified,unknown --fail --fail-on-scan-errors filesystem ",
|
||||
);
|
||||
});
|
||||
|
||||
it("scans the staged index snapshot before the first commit", () => {
|
||||
const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-first-");
|
||||
run(dir, "git", ["init", "-q", "--initial-branch=main"]);
|
||||
const fakeBinDir = installPreCommitFixture(dir);
|
||||
const logPath = path.join(dir, "trufflehog.log");
|
||||
writeExecutable(
|
||||
fakeBinDir,
|
||||
"trufflehog",
|
||||
`#!/usr/bin/env bash
|
||||
printf 'env=%s\n' "\${TRUFFLEHOG_PRE_COMMIT:-}" > ${JSON.stringify(logPath)}
|
||||
printf 'args=%s\n' "$*" >> ${JSON.stringify(logPath)}
|
||||
`,
|
||||
);
|
||||
|
||||
writeFileSync(path.join(dir, "changed.txt"), "safe staged content\n", "utf8");
|
||||
run(dir, "git", ["add", "--", "changed.txt"]);
|
||||
|
||||
run(dir, "bash", ["git-hooks/pre-commit"], {
|
||||
PATH: `${fakeBinDir}:${process.env.PATH ?? ""}`,
|
||||
});
|
||||
|
||||
const log = readFileSync(logPath, "utf8");
|
||||
expect(log).toContain("env=\n");
|
||||
expect(log).toContain(
|
||||
"args=--no-update --no-color --results=verified,unknown --fail --fail-on-scan-errors filesystem ",
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks with install guidance when TruffleHog is missing", () => {
|
||||
const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-no-trufflehog-");
|
||||
run(dir, "git", ["init", "-q", "--initial-branch=main"]);
|
||||
const fakeBinDir = installPreCommitFixture(dir);
|
||||
run(dir, "rm", ["-f", path.join(fakeBinDir, "trufflehog")]);
|
||||
|
||||
writeFileSync(path.join(dir, "changed.txt"), "safe staged content\n", "utf8");
|
||||
run(dir, "git", ["add", "--", "changed.txt"]);
|
||||
|
||||
const failure = runFailure(dir, "bash", ["git-hooks/pre-commit"], {
|
||||
PATH: `${fakeBinDir}:/usr/bin:/bin`,
|
||||
});
|
||||
|
||||
expect(failure.status).toBe(1);
|
||||
expect(failure.stderr).toContain(
|
||||
"OpenClaw requires TruffleHog for pre-commit secret scanning.",
|
||||
);
|
||||
expect(failure.stderr).toContain("brew install trufflehog");
|
||||
expect(failure.stderr).toContain("https://github.com/trufflesecurity/trufflehog#installation");
|
||||
});
|
||||
|
||||
it("does not run the changed-scope check for non-doc staged changes", () => {
|
||||
const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-no-check-changed-");
|
||||
run(dir, "git", ["init", "-q", "--initial-branch=main"]);
|
||||
|
||||
Reference in New Issue
Block a user