Skip to content

Commit 86bab96

Browse files
pgondhi987pgondhi987
authored
fix: block git protocol env controls [AI] (#91619)
* fix: block git protocol env controls * fix: preserve restrictive git protocol env * fix: preserve restrictive git allowlists * fix: filter inherited git protocol allowlists * test: cover restrictive git allowlists * test: avoid opengrep fixture false positives * test: type env fixture helper narrowly * fix: preserve zero git protocol booleans * fix: preserve invalid git protocol booleans * fix: force git protocol from user off * fix: share git inherited env sanitization
1 parent d2a6529 commit 86bab96

9 files changed

Lines changed: 667 additions & 220 deletions

‎apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ enum HostEnvSanitizer {
2424
"NO_COLOR",
2525
"FORCE_COLOR",
2626
]
27+
private static let gitAllowProtocolKey = "GIT_ALLOW_PROTOCOL"
28+
private static let gitProtocolFromUserKey = "GIT_PROTOCOL_FROM_USER"
29+
private static let gitProtocolFromUserDisabledValue = "0"
30+
private static let gitDefaultAlwaysAllowedProtocols: Set<String> = [
31+
"git",
32+
"http",
33+
"https",
34+
"ssh",
35+
]
2736

2837
private static func isBlocked(_ upperKey: String) -> Bool {
2938
if self.blockedKeys.contains(upperKey) { return true }
@@ -82,6 +91,25 @@ enum HostEnvSanitizer {
8291
Array(Set(values)).sorted()
8392
}
8493

94+
private static func isPermissiveGitProtocolFromUserValue(_ value: String) -> Bool {
95+
let normalized = value.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()
96+
if normalized == "true" || normalized == "yes" || normalized == "on" {
97+
return true
98+
}
99+
let isInteger = normalized.range(of: #"^[+-]?[0-9]+$"#, options: .regularExpression) != nil
100+
let isZero = normalized.range(of: #"^[+-]?0+$"#, options: .regularExpression) != nil
101+
return isInteger && !isZero
102+
}
103+
104+
private static func sanitizeInheritedGitAllowProtocolValue(_ value: String) -> String {
105+
let normalized = value.trimmingCharacters(in: .whitespacesAndNewlines)
106+
if normalized.isEmpty { return "" }
107+
let safeProtocols = normalized
108+
.split(separator: ":", omittingEmptySubsequences: false)
109+
.filter { self.gitDefaultAlwaysAllowedProtocols.contains(String($0)) }
110+
return safeProtocols.joined(separator: ":")
111+
}
112+
85113
static func inspectOverrides(
86114
overrides: [String: String]?,
87115
blockPathOverrides: Bool = true) -> HostEnvOverrideDiagnostics
@@ -120,6 +148,22 @@ enum HostEnvSanitizer {
120148
let key = rawKey.trimmingCharacters(in: .whitespacesAndNewlines)
121149
guard !key.isEmpty else { continue }
122150
let upper = key.uppercased()
151+
// Preserve inherited Git allowlists without widening malformed or unsafe entries by
152+
// deletion. Protocols outside Git's safe default set are removed instead.
153+
if upper == self.gitAllowProtocolKey {
154+
merged[key] = self.sanitizeInheritedGitAllowProtocolValue(value)
155+
continue
156+
}
157+
// Preserve non-permissive Git boolean values. Permissive values must become explicit
158+
// `0` because Git's unset default still permits protocols with policy `user`.
159+
if upper == self.gitProtocolFromUserKey {
160+
if !self.isPermissiveGitProtocolFromUserValue(value) {
161+
merged[key] = value
162+
} else {
163+
merged[key] = self.gitProtocolFromUserDisabledValue
164+
}
165+
continue
166+
}
123167
if self.isBlockedInherited(upper) { continue }
124168
merged[key] = value
125169
}

‎apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ enum HostEnvSecurityPolicy {
7878
"GH_TOKEN",
7979
"GITHUB_TOKEN",
8080
"GITLAB_TOKEN",
81+
"GIT_ALLOW_PROTOCOL",
8182
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
8283
"GIT_ASKPASS",
8384
"GIT_COMMON_DIR",
@@ -89,6 +90,7 @@ enum HostEnvSecurityPolicy {
8990
"GIT_INDEX_FILE",
9091
"GIT_NAMESPACE",
9192
"GIT_OBJECT_DIRECTORY",
93+
"GIT_PROTOCOL_FROM_USER",
9294
"GIT_PROXY_COMMAND",
9395
"GIT_SEQUENCE_EDITOR",
9496
"GIT_SSH",
@@ -239,6 +241,7 @@ enum HostEnvSecurityPolicy {
239241
"EXINIT",
240242
"FPATH",
241243
"GCONV_PATH",
244+
"GIT_ALLOW_PROTOCOL",
242245
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
243246
"GIT_COMMON_DIR",
244247
"GIT_DIR",
@@ -249,6 +252,7 @@ enum HostEnvSecurityPolicy {
249252
"GIT_INDEX_FILE",
250253
"GIT_NAMESPACE",
251254
"GIT_OBJECT_DIRECTORY",
255+
"GIT_PROTOCOL_FROM_USER",
252256
"GIT_SEQUENCE_EDITOR",
253257
"GIT_SSL_CAINFO",
254258
"GIT_SSL_CAPATH",

‎src/agents/bash-tools.exec-runtime.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ let formatExecFailureReason: typeof import("./bash-tools.exec-runtime.js").forma
3434
let renderExecUpdateText: typeof import("./bash-tools.exec-runtime.js").renderExecUpdateText;
3535
let resolveExecTarget: typeof import("./bash-tools.exec-runtime.js").resolveExecTarget;
3636
let runExecProcess: typeof import("./bash-tools.exec-runtime.js").runExecProcess;
37+
let sanitizeHostBaseEnv: typeof import("./bash-tools.exec-runtime.js").sanitizeHostBaseEnv;
3738

3839
beforeAll(async () => {
3940
({ markBackgrounded } = await import("./bash-process-registry.js"));
@@ -45,6 +46,7 @@ beforeAll(async () => {
4546
renderExecUpdateText,
4647
resolveExecTarget,
4748
runExecProcess,
49+
sanitizeHostBaseEnv,
4850
} = await import("./bash-tools.exec-runtime.js"));
4951
});
5052

@@ -113,6 +115,33 @@ describe("detectCursorKeyMode", () => {
113115
});
114116
});
115117

118+
describe("sanitizeHostBaseEnv", () => {
119+
it("uses value-aware Git protocol inherited env sanitization", () => {
120+
expect(
121+
sanitizeHostBaseEnv({
122+
PATH: "/usr/bin:/bin",
123+
GIT_ALLOW_PROTOCOL: "https:ext:ssh",
124+
GIT_PROTOCOL_FROM_USER: "1",
125+
GIT_SSH_COMMAND: "touch /tmp/pwned",
126+
SAFE: "ok",
127+
}),
128+
).toEqual({
129+
PATH: "/usr/bin:/bin",
130+
GIT_ALLOW_PROTOCOL: "https:ssh",
131+
GIT_PROTOCOL_FROM_USER: "0",
132+
SAFE: "ok",
133+
});
134+
135+
expect(
136+
sanitizeHostBaseEnv({
137+
GIT_PROTOCOL_FROM_USER: "false",
138+
}),
139+
).toEqual({
140+
GIT_PROTOCOL_FROM_USER: "false",
141+
});
142+
});
143+
});
144+
116145
describe("resolveExecTarget", () => {
117146
it("keeps implicit auto on sandbox when a sandbox runtime is available", () => {
118147
expectExecTarget(

‎src/agents/bash-tools.exec-runtime.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import {
1919
type ExecTarget,
2020
} from "../infra/exec-approvals.js";
2121
import { requestHeartbeat } from "../infra/heartbeat-wake.js";
22-
import { isDangerousHostInheritedEnvVarName } from "../infra/host-env-security.js";
22+
import {
23+
isDangerousHostInheritedEnvVarName,
24+
sanitizeHostInheritedEnvEntry,
25+
} from "../infra/host-env-security.js";
2326
import { findPathKey, mergePathPrepend, removePathPrepend } from "../infra/path-prepend.js";
2427
import { enqueueSystemEvent } from "../infra/system-events.js";
2528
import { isSubagentSessionKey } from "../sessions/session-key-utils.js";
@@ -93,15 +96,12 @@ export function detectCursorKeyMode(raw: string): "application" | "normal" | nul
9396
export function sanitizeHostBaseEnv(env: Record<string, string>): Record<string, string> {
9497
const sanitized: Record<string, string> = {};
9598
for (const [key, value] of Object.entries(env)) {
96-
const upperKey = key.toUpperCase();
97-
if (upperKey === "PATH") {
98-
sanitized[key] = value;
99-
continue;
100-
}
101-
if (isDangerousHostInheritedEnvVarName(upperKey)) {
99+
const sanitizedEntry = sanitizeHostInheritedEnvEntry(key, value);
100+
if (!sanitizedEntry) {
102101
continue;
103102
}
104-
sanitized[key] = value;
103+
const [sanitizedKey, sanitizedValue] = sanitizedEntry;
104+
sanitized[sanitizedKey] = sanitizedValue;
105105
}
106106
return sanitized;
107107
}

‎src/infra/host-env-security-policy.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"ENV",
1818
"KSH_ENV",
1919
"BROWSER",
20+
"GIT_ALLOW_PROTOCOL",
2021
"GIT_EDITOR",
2122
"GIT_EXTERNAL_DIFF",
2223
"GIT_DIR",
@@ -27,6 +28,7 @@
2728
"GIT_OBJECT_DIRECTORY",
2829
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
2930
"GIT_NAMESPACE",
31+
"GIT_PROTOCOL_FROM_USER",
3032
"GIT_SEQUENCE_EDITOR",
3133
"GIT_TEMPLATE_DIR",
3234
"GIT_SSL_NO_VERIFY",

‎src/infra/host-env-security.reported-baseline.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"EXINIT",
3232
"FPATH",
3333
"GCONV_PATH",
34+
"GIT_ALLOW_PROTOCOL",
3435
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
3536
"GIT_COMMON_DIR",
3637
"GIT_DIR",
@@ -41,6 +42,7 @@
4142
"GIT_INDEX_FILE",
4243
"GIT_NAMESPACE",
4344
"GIT_OBJECT_DIRECTORY",
45+
"GIT_PROTOCOL_FROM_USER",
4446
"GIT_SEQUENCE_EDITOR",
4547
"GIT_SSL_CAINFO",
4648
"GIT_SSL_CAPATH",
@@ -257,5 +259,5 @@
257259
"YARN_RC_FILENAME",
258260
"ZDOTDIR"
259261
],
260-
"expectedTotalReportedEntries": 252
262+
"expectedTotalReportedEntries": 254
261263
}

‎src/infra/host-env-security.reported-baseline.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe("host env reported baseline coverage", () => {
9797
baseline.reportedDangerousEverywhereKeys.length +
9898
baseline.reportedDangerousOverrideOnlyKeys.length,
9999
).toBe(baseline.expectedTotalReportedEntries);
100-
expect(baseline.expectedTotalReportedEntries).toBe(252);
100+
expect(baseline.expectedTotalReportedEntries).toBe(254);
101101
expect(sortUniqueUpper(baseline.reportedDangerousEverywhereKeys)).toEqual(
102102
baseline.reportedDangerousEverywhereKeys,
103103
);
@@ -119,6 +119,14 @@ describe("host env reported baseline coverage", () => {
119119
for (const key of baseline.reportedDangerousEverywhereKeys) {
120120
expect(isDangerousHostEnvVarName(key)).toBe(true);
121121
expect(isDangerousHostInheritedEnvVarName(key)).toBe(true);
122+
if (key === "GIT_ALLOW_PROTOCOL") {
123+
expect(inheritedSanitized[key]).toBe("");
124+
continue;
125+
}
126+
if (key === "GIT_PROTOCOL_FROM_USER") {
127+
expect(inheritedSanitized[key]).toBe(`${key.toLowerCase()}-from-inherited`);
128+
continue;
129+
}
122130
expect(inheritedSanitized[key]).toBeUndefined();
123131
}
124132

0 commit comments

Comments
 (0)