Skip to content

Commit 33b147a

Browse files
authored
fix(cli): reject destroy view path-join escape
CLI Hardener pin. S1 destroy view path-escape FIX. S2/S6–S8 PROVEN. S3/S4/S5/S9 HOLD unflipped (doctor/analyze/start exit 0; JDBC pin fork).
1 parent 77f9f51 commit 33b147a

6 files changed

Lines changed: 295 additions & 2 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `wheels destroy view` now rejects path-join escapes such as `../x` so deletes stay under `app/views/`

cli/lucli/services/Destroy.cfc

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ component {
151151
if (find("/", arguments.name)) {
152152
// Single view file: "products/index"
153153
var parts = listToArray(arguments.name, "/");
154-
if (arrayLen(parts) != 2 || !len(parts[1]) || !len(parts[2])) {
154+
if (arrayLen(parts) != 2 || !len(parts[1]) || !len(parts[2]) || $viewPathEscapes(arguments.name)) {
155155
result.success = false;
156156
result.warnings = ["Invalid view path. Use: controller/viewname (e.g., products/index)"];
157157
return result;
@@ -212,7 +212,7 @@ component {
212212
case "view":
213213
if (find("/", arguments.name)) {
214214
var parts = listToArray(arguments.name, "/");
215-
if (arrayLen(parts) == 2 && len(parts[1]) && len(parts[2])) {
215+
if (arrayLen(parts) == 2 && len(parts[1]) && len(parts[2]) && !$viewPathEscapes(arguments.name)) {
216216
arrayAppend(preview, "app/views/" & parts[1] & "/" & parts[2] & ".cfm");
217217
} else {
218218
arrayAppend(preview, "Invalid view path: " & arguments.name);
@@ -247,6 +247,20 @@ component {
247247
};
248248
}
249249

250+
/**
251+
* True when a controller/view token would path-join outside app/views/
252+
* (e.g. `../x`, `foo/../bar`, Windows `..\\x`).
253+
*/
254+
private boolean function $viewPathEscapes(required string name) {
255+
if (find("..", arguments.name)) {
256+
return true;
257+
}
258+
if (find(chr(92), arguments.name)) {
259+
return true;
260+
}
261+
return false;
262+
}
263+
250264
private void function deleteFileIfExists(required string path, required struct result) {
251265
if (fileExists(arguments.path)) {
252266
fileDelete(arguments.path);
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/**
2+
* CLI Hardener S1–S10 (LuCLI seam).
3+
*
4+
* S1 FIX lives in Destroy.cfc / DestroySpec (path-join escape).
5+
* This spec pins HOLD S3/S4/S5/S9 and proves S2/S7/S8 at source + evaluator
6+
* altitude. No exit-code flips. No SQLite pin unification.
7+
*/
8+
component extends="wheels.wheelstest.system.BaseSpec" {
9+
10+
function beforeAll() {
11+
variables.repoRoot = expandPath("/cli/../");
12+
variables.moduleSrc = fileRead(expandPath("/cli/lucli/Module.cfc"));
13+
variables.cliLocalScript = variables.repoRoot & "tools/test-cli-local.sh";
14+
variables.ciRunTests = variables.repoRoot & "tools/ci/run-tests.sh";
15+
variables.prYml = variables.repoRoot & ".github/workflows/pr.yml";
16+
}
17+
18+
function run() {
19+
20+
describe("S2 PROVE — generate controller ../X never hits validateName", () => {
21+
22+
it("Module.generateController does not call validateName before CodeGen", () => {
23+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*private\s+string\s+function\s+generateController\s*\(", 500);
24+
expect(findNoCase("validateName", body)).toBe(0);
25+
expect(body).toInclude("codegen.generateController");
26+
});
27+
28+
});
29+
30+
describe("S3 HOLD — wheels doctor CRITICAL then return empty string", () => {
31+
32+
it("doctor() prints CRITICAL then returns empty string and does not throw", () => {
33+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*public\s+string\s+function\s+doctor\s*\(", 6000);
34+
expect(body).toInclude('case "CRITICAL"');
35+
expect(body).toInclude("Status: CRITICAL");
36+
expect(body).toInclude("return """"");
37+
expect(findNoCase("throw(", body)).toBe(0);
38+
expect(findNoCase("rethrow", body)).toBe(0);
39+
});
40+
41+
it("validate() already throws Wheels.ValidationFailed (contrast, not flipped)", () => {
42+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*public\s+string\s+function\s+validate\s*\(", 3000);
43+
expect(body).toInclude("Wheels.ValidationFailed");
44+
expect(body).toInclude("rethrow");
45+
});
46+
47+
});
48+
49+
describe("S4 HOLD — wheels analyze catch any then return empty string", () => {
50+
51+
it("analyze() swallows catch (any) and still returns empty string", () => {
52+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*public\s+string\s+function\s+analyze\s*\(", 6000);
53+
expect(body).toInclude("catch (any e)");
54+
expect(body).toInclude("Analysis failed:");
55+
expect(body).toInclude("return """"");
56+
expect(findNoCase("rethrow", body)).toBe(0);
57+
});
58+
59+
});
60+
61+
describe("S5 HOLD — wheels start refuse paths return empty string", () => {
62+
63+
it("start() not-a-project and name-collision refuses return empty string", () => {
64+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*public\s+string\s+function\s+start\s*\(", 8000);
65+
expect(body).toInclude("$isWheelsProjectDir");
66+
expect(body).toInclude("!reg.ours");
67+
expect(body).toInclude("return """"");
68+
expect(findNoCase("throw(", body)).toBe(0);
69+
});
70+
71+
});
72+
73+
describe("S7 PROVE — TestRunner.runViaHttp is a mirrored helper, not live wheels test", () => {
74+
75+
it("$buildTestRunnerPath is app|core only — no /wheels/cli/tests", () => {
76+
var body = $sliceFn(moduleSrc, "(?m)^[ \t]*public\s+string\s+function\s+\$buildTestRunnerPath\s*\(", 500);
77+
expect(body).toInclude("/wheels/core/tests");
78+
expect(body).toInclude("/wheels/app/tests");
79+
expect(findNoCase("/wheels/cli/tests", body)).toBe(0);
80+
});
81+
82+
it("CLI runner emits 417 on Fail/Error; test-cli-local.sh accepts 417 as a payload", () => {
83+
var runner = fileRead(expandPath("/cli/lucli/tests/runner.cfm"));
84+
expect(runner).toInclude("statuscode = 417");
85+
var sh = fileRead(cliLocalScript);
86+
expect(sh).toInclude("/wheels/cli/tests");
87+
expect(sh).toInclude('[ "$HTTP_CODE" = "200" ] || [ "$HTTP_CODE" = "417" ]');
88+
});
89+
90+
});
91+
92+
describe("S8 PROVE — test-cli-local.sh STRICT default is 0; CI is strict", () => {
93+
94+
it("pins WHEELS_CLI_TEST_STRICT default 0 and does not flip it to 1", () => {
95+
var sh = fileRead(cliLocalScript);
96+
expect(sh).toInclude('WHEELS_CLI_TEST_STRICT="${WHEELS_CLI_TEST_STRICT:-0}"');
97+
expect(find('WHEELS_CLI_TEST_STRICT="${WHEELS_CLI_TEST_STRICT:-1}"', sh)).toBe(0);
98+
expect(sh).toInclude("os.environ.get('WHEELS_CLI_TEST_STRICT', '0') == '1'");
99+
});
100+
101+
it("CI tools/ci/run-tests.sh fail-closes on any CLI Fail/Error", () => {
102+
var ci = fileRead(ciRunTests);
103+
expect(ci).toInclude("CLI_TOTAL_FAILURES");
104+
expect(ci).toInclude('elif [ "$CLI_TOTAL_FAILURES" -gt 0 ]');
105+
expect(ci).toInclude("CLI_OK=false");
106+
expect(ci).toInclude('if [ "$CORE_OK" = false ] || [ "$CLI_OK" = false ]; then');
107+
});
108+
109+
it("STRICT=0 exits 0 on a non-deploy fail; STRICT=1 fails closed", () => {
110+
var mockPath = getTempDirectory() & "cli-strict-nongate-" & createUUID() & ".json";
111+
fileWrite(mockPath, $nongatingFailJson());
112+
var loose = $evalCliLocalStrict(mockJsonPath = mockPath, strictFlag = "0");
113+
var tight = $evalCliLocalStrict(mockJsonPath = mockPath, strictFlag = "1");
114+
if (fileExists(mockPath)) {
115+
fileDelete(mockPath);
116+
}
117+
expect(loose).toBe(0);
118+
expect(tight).toBe(1);
119+
});
120+
121+
it("STRICT=0 still gates a deploy-bundle fail (default is not 'always 0')", () => {
122+
var mockPath = getTempDirectory() & "cli-strict-deploy-" & createUUID() & ".json";
123+
fileWrite(mockPath, $deployFailJson());
124+
var code = $evalCliLocalStrict(mockJsonPath = mockPath, strictFlag = "0");
125+
if (fileExists(mockPath)) {
126+
fileDelete(mockPath);
127+
}
128+
expect(code).toBe(1);
129+
});
130+
131+
});
132+
133+
describe("S9 HOLD — SQLite JDBC pins stay forked", () => {
134+
135+
it("wheels start stages 3.49.1.0; test-cli-local.sh downloads 3.49.1.0; CI pr.yml uses 3.50.3.0", () => {
136+
expect(moduleSrc).toInclude("sqlite-jdbc-3.49.1.0");
137+
expect(find("3.50.3.0", moduleSrc)).toBe(0);
138+
var sh = fileRead(cliLocalScript);
139+
expect(sh).toInclude("sqlite-jdbc/3.49.1.0/sqlite-jdbc-3.49.1.0.jar");
140+
expect(find("3.50.3.0", sh)).toBe(0);
141+
var pr = fileRead(prYml);
142+
expect(pr).toInclude("sqlite-jdbc/3.50.3.0/sqlite-jdbc-3.50.3.0.jar");
143+
expect(find("3.49.1.0", pr)).toBe(0);
144+
});
145+
146+
});
147+
148+
}
149+
150+
private string function $sliceFn(required string src, required string pattern, numeric window = 800) {
151+
var startIdx = reFindNoCase(arguments.pattern, arguments.src);
152+
expect(startIdx).toBeGT(0);
153+
var chunk = mid(arguments.src, startIdx, arguments.window);
154+
// Trim at the next top-level function so a generous window cannot
155+
// leak the following method (e.g. analyze() into validate()).
156+
var nextFn = reFindNoCase("(?m)^[ \t]*(public|private)\s+\w+\s+function\s+", chunk, 2);
157+
if (isArray(nextFn)) {
158+
nextFn = arrayLen(nextFn) ? nextFn[1] : 0;
159+
}
160+
if (nextFn > 1) {
161+
chunk = left(chunk, nextFn - 1);
162+
}
163+
return chunk;
164+
}
165+
166+
private string function $nongatingFailJson() {
167+
return '{"totalPass":10,"totalFail":1,"totalError":0,"bundleStats":[{"name":"cli.lucli.tests.specs.services.FooSpec","suiteStats":[{"specStats":[{"name":"fails on purpose","status":"Failed","failMessage":"boom"}]}]}]}';
168+
}
169+
170+
private string function $deployFailJson() {
171+
return '{"totalPass":10,"totalFail":1,"totalError":0,"bundleStats":[{"name":"cli.lucli.tests.specs.deploy.cli.DeployMainCliSpec","suiteStats":[{"specStats":[{"name":"deploy fail","status":"Failed","failMessage":"boom"}]}]}]}';
172+
}
173+
174+
private numeric function $evalCliLocalStrict(required string mockJsonPath, required string strictFlag) {
175+
var shSrc = fileRead(variables.cliLocalScript);
176+
var importAt = find("import json, os, sys", shSrc);
177+
var exitNeedle = "sys.exit(0 if gating_failures == 0 else 1)";
178+
var exitAt = find(exitNeedle, shSrc);
179+
expect(importAt).toBeGT(0);
180+
expect(exitAt).toBeGT(0);
181+
var inner = mid(shSrc, importAt, exitAt + len(exitNeedle) - importAt);
182+
183+
var bs = chr(92);
184+
var q = chr(34);
185+
inner = replace(inner, "$RESULT_FILE", arguments.mockJsonPath, "all");
186+
inner = replace(inner, bs & q, q, "all");
187+
inner = replace(inner, bs & bs & "n", bs & "n", "all");
188+
189+
var pyPath = getTempDirectory() & "cli-strict-eval-" & createUUID() & ".py";
190+
fileWrite(pyPath, inner);
191+
192+
var cmd = createObject("java", "java.util.ArrayList").init();
193+
cmd.add("/usr/bin/python3");
194+
cmd.add(pyPath);
195+
var pb = createObject("java", "java.lang.ProcessBuilder").init(cmd);
196+
pb.redirectErrorStream(true);
197+
pb.environment().put("WHEELS_CLI_TEST_STRICT", arguments.strictFlag);
198+
var proc = pb.start();
199+
proc.getInputStream().readAllBytes();
200+
proc.waitFor();
201+
var code = proc.exitValue();
202+
if (fileExists(pyPath)) {
203+
fileDelete(pyPath);
204+
}
205+
return code;
206+
}
207+
208+
}

cli/lucli/tests/specs/services/CodeGenSpec.cfc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,18 @@ component extends="wheels.wheelstest.system.BaseSpec" {
5151
expect(find("SENTINEL", fileRead(path))).toBe(0);
5252
});
5353

54+
it("S6 PROVE: missing-template fallback mints expect(true).toBeTrue()", () => {
55+
var result = codegen.generateTest(
56+
type = "noSuchTemplateType",
57+
name = "S6FallbackMint",
58+
force = true
59+
);
60+
expect(result.success).toBeTrue();
61+
expect(result.message).toInclude("inline template");
62+
var content = fileRead(tempRoot & "/tests/specs/unit/S6FallbackMintSpec.cfc");
63+
expect(content).toInclude("expect(true).toBeTrue();");
64+
});
65+
5466
});
5567

5668
describe("generateModel()", () => {
@@ -265,6 +277,25 @@ component extends="wheels.wheelstest.system.BaseSpec" {
265277
expect(result.actions).toBeEmpty();
266278
});
267279

280+
it("S2 PROVE: packagePath from listFirst is unvalidated so ../X writes outside app/controllers/", () => {
281+
// Current hole: listFirst("../S2Escape","/") is ".." and is
282+
// joined as packagePath without validateName. Destination
283+
// becomes app/controllers/../S2Escape.cfc → app/S2Escape.cfc.
284+
var result = codegen.generateController(
285+
name = "../S2Escape",
286+
actions = [],
287+
force = true
288+
);
289+
var escapedPath = tempRoot & "/app/S2Escape.cfc";
290+
var controllersPath = tempRoot & "/app/controllers/S2Escape.cfc";
291+
expect(result.success).toBeTrue();
292+
expect(fileExists(escapedPath)).toBeTrue();
293+
expect(fileExists(controllersPath)).toBeFalse();
294+
if (fileExists(escapedPath)) {
295+
fileDelete(escapedPath);
296+
}
297+
});
298+
268299
});
269300

270301
describe("generatePolicy()", () => {
@@ -333,6 +364,11 @@ component extends="wheels.wheelstest.system.BaseSpec" {
333364
expect(result.valid).toBeTrue();
334365
});
335366

367+
it("S2 PROVE: validateName rejects ../X but generateController never consults it", () => {
368+
var result = codegen.validateName("../X", "controller");
369+
expect(result.valid).toBeFalse();
370+
});
371+
336372
});
337373

338374
});

cli/lucli/tests/specs/services/DestroySpec.cfc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,22 @@ component extends="wheels.wheelstest.system.BaseSpec" {
185185
expect(result.success).toBeFalse();
186186
});
187187

188+
it("S1 FIX: rejects ../x so the join cannot leave app/views/", () => {
189+
var outside = tempRoot & "/app/S1Escape.cfm";
190+
fileWrite(outside, "should-not-delete");
191+
var result = destroy.destroyView("../S1Escape");
192+
expect(result.success).toBeFalse();
193+
expect(fileExists(outside)).toBeTrue();
194+
expect(arrayToList(result.warnings)).toInclude("Invalid view path");
195+
fileDelete(outside);
196+
});
197+
198+
it("S1 FIX: rejects a .. segment in either half of controller/view", () => {
199+
var result = destroy.destroyView("products/..");
200+
expect(result.success).toBeFalse();
201+
expect(arrayToList(result.warnings)).toInclude("Invalid view path");
202+
});
203+
188204
});
189205

190206
describe("previewDestroy()", () => {
@@ -197,6 +213,12 @@ component extends="wheels.wheelstest.system.BaseSpec" {
197213
expect(arrayToList(preview)).toInclude("drop table");
198214
});
199215

216+
it("S1 FIX: previewDestroy does not join ../x under app/views/", () => {
217+
var preview = destroy.previewDestroy("../S1Escape", "view");
218+
expect(arrayToList(preview)).notToInclude("app/views/../");
219+
expect(arrayToList(preview)).toInclude("Invalid view path");
220+
});
221+
200222
it("returns controller and spec only — views excluded (##2493)", () => {
201223
// Type-scoped controller destroy is narrow: only the
202224
// controller .cfc and its spec. Views are explicitly NOT

cli/lucli/tests/specs/services/TestRunnerSpec.cfc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,18 @@ component extends="wheels.wheelstest.system.BaseSpec" {
7979
expect(structKeyExists(result, "message")).toBeTrue();
8080
});
8181

82+
it("S7 PROVE: mirrored helper only accepts HTTP 200 and never hits /wheels/cli/tests", () => {
83+
var src = fileRead(expandPath("/cli/lucli/services/TestRunner.cfc"));
84+
var startIdx = reFindNoCase("(?m)^[ \t]*public\s+struct\s+function\s+runViaHttp\s*\(", src);
85+
expect(startIdx).toBeGT(0);
86+
var body = mid(src, startIdx, 1200);
87+
expect(body).toInclude("/wheels/core/tests");
88+
expect(body).toInclude("/wheels/app/tests");
89+
expect(findNoCase("/wheels/cli/tests", body)).toBe(0);
90+
expect(body).toInclude('statusCode contains "200"');
91+
expect(find('contains "417"', body)).toBe(0);
92+
});
93+
8294
});
8395

8496
describe("countSpecsOnDisk()", () => {

0 commit comments

Comments
 (0)