Skip to content

Commit f55f3a8

Browse files
committed
fix(windows): capture SVN status without shell
Add an isolated direct-argv stdout capture path for Windows and use it for pinned SVN status probes. Preserve literal metacharacters, process exit codes, and the existing restricted handle inheritance model. Signed-off-by: lzl51230 <14934913+lzl51230@users.noreply.github.com>
1 parent 65743ba commit f55f3a8

6 files changed

Lines changed: 96 additions & 20 deletions

File tree

src/foundation/compat_fs.c

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ static wchar_t *cbm_resolve_comspec(void) {
192192

193193
/* On failure returns NULL with *stage naming the failing step and *gle the
194194
* GetLastError value captured at that step (0 when errno is the signal). */
195-
static FILE *cbm_popen_isolated(const char *cmd, const char **stage, DWORD *gle) {
195+
static FILE *cbm_popen_isolated(const char *cmd, const char *const *argv, const char **stage,
196+
DWORD *gle) {
196197
*stage = "";
197198
*gle = 0;
198199
InitOnceExecuteOnce(&g_popen_once, cbm_popen_init, NULL, NULL);
@@ -246,12 +247,19 @@ static FILE *cbm_popen_isolated(const char *cmd, const char **stage, DWORD *gle)
246247
si.StartupInfo.hStdError = nul;
247248
si.lpAttributeList = attr;
248249

249-
/* Run through cmd.exe /c so command quoting and `2>NUL` behave as under
250-
* _popen. The command line is heap-composed (no fixed-size truncation)
251-
* and widened via UTF-8 so non-ASCII repo paths survive intact. */
252-
wchar_t *app = cbm_resolve_comspec();
250+
/* Shell commands run through cmd.exe /c for _popen compatibility. Argv
251+
* callers bypass the shell and pin the requested executable directly.
252+
* Both paths use heap-composed UTF-8 command lines so non-ASCII paths
253+
* survive intact. */
254+
wchar_t *app = NULL;
253255
wchar_t *wcmdline = NULL;
254-
if (app) {
256+
if (argv) {
257+
app = cbm_utf8_to_wide(argv[0]);
258+
wcmdline = cbm_build_cmdline(argv);
259+
} else {
260+
app = cbm_resolve_comspec();
261+
}
262+
if (app && !argv) {
255263
size_t u8len = strlen(cmd) + sizeof("cmd.exe /c ");
256264
char *u8 = (char *)malloc(u8len);
257265
if (u8) {
@@ -334,7 +342,7 @@ FILE *cbm_popen(const char *cmd, const char *mode) {
334342
if (mode && mode[0] == 'r' && mode[1] == '\0') {
335343
const char *stage = "";
336344
DWORD gle = 0;
337-
FILE *fp = cbm_popen_isolated(cmd, &stage, &gle);
345+
FILE *fp = cbm_popen_isolated(cmd, NULL, &stage, &gle);
338346
g_popen_last_isolated = (fp != NULL);
339347
if (!fp) {
340348
char glebuf[CBM_SZ_16];
@@ -350,6 +358,24 @@ FILE *cbm_popen(const char *cmd, const char *mode) {
350358
return _popen(cmd, mode);
351359
}
352360

361+
FILE *cbm_popen_argv(const char *const *argv) {
362+
if (!argv || !argv[0] || !argv[0][0]) {
363+
return NULL;
364+
}
365+
const char *stage = "";
366+
DWORD gle = 0;
367+
FILE *fp = cbm_popen_isolated(NULL, argv, &stage, &gle);
368+
g_popen_last_isolated = (fp != NULL);
369+
if (!fp) {
370+
char glebuf[CBM_SZ_16];
371+
char errnobuf[CBM_SZ_16];
372+
snprintf(glebuf, sizeof(glebuf), "%lu", (unsigned long)gle);
373+
snprintf(errnobuf, sizeof(errnobuf), "%d", errno);
374+
cbm_log_warn("compat.popen_argv_failed", "stage", stage, "gle", glebuf, "errno", errnobuf);
375+
}
376+
return fp;
377+
}
378+
353379
int cbm_pclose(FILE *f) {
354380
InitOnceExecuteOnce(&g_popen_once, cbm_popen_init, NULL, NULL);
355381

src/foundation/compat_fs.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ void cbm_closedir(cbm_dir_t *d);
3838

3939
FILE *cbm_popen(const char *cmd, const char *mode);
4040
int cbm_pclose(FILE *f);
41+
#ifdef _WIN32
42+
/* Spawn an argv vector directly while capturing stdout through the same
43+
* isolated handle-inheritance path as cbm_popen. */
44+
FILE *cbm_popen_argv(const char *const *argv);
45+
#endif
4146

4247
/* ── File operations ──────────────────────────────────────────── */
4348

src/watcher/svn_state.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,22 +1019,27 @@ cbm_svn_probe_result_t cbm_svn_client_init(const char *root_path, cbm_svn_client
10191019

10201020
cbm_svn_probe_result_t cbm_svn_probe(const cbm_svn_client_t *client, const char *root_path,
10211021
cbm_svn_observation_t *observation) {
1022-
if (!client || !client->executable[0] || !root_path || !observation ||
1023-
!cbm_validate_shell_arg(client->executable) || !cbm_validate_shell_arg(root_path)) {
1022+
if (!client || !client->executable[0] || !root_path || !observation) {
10241023
return CBM_SVN_PROBE_UNCERTAIN;
10251024
}
10261025
char status_root[CBM_SZ_4K];
1027-
if (!cbm_svn_format_path_arg(client, root_path, status_root, sizeof(status_root)) ||
1028-
!cbm_validate_shell_arg(status_root)) {
1026+
if (!cbm_svn_format_path_arg(client, root_path, status_root, sizeof(status_root))) {
10291027
return CBM_SVN_PROBE_UNCERTAIN;
10301028
}
10311029
#ifdef _WIN32
1032-
/* cbm_popen routes through cmd.exe, where percent expansion occurs even
1033-
* inside double quotes. Fail closed until this leaf can use argv capture. */
1034-
if (strchr(client->executable, '%') || strchr(status_root, '%')) {
1030+
char peg_root[CBM_SZ_8K];
1031+
int peg_length = snprintf(peg_root, sizeof(peg_root), "%s@", status_root);
1032+
if (peg_length < 0 || (size_t)peg_length >= sizeof(peg_root)) {
1033+
return CBM_SVN_PROBE_UNCERTAIN;
1034+
}
1035+
const char *argv[] = {
1036+
client->executable, "status", "--xml", "--verbose", "--no-ignore", "--depth", "infinity",
1037+
"--non-interactive", "--", peg_root, NULL};
1038+
FILE *process = cbm_popen_argv(argv);
1039+
#else
1040+
if (!cbm_validate_shell_arg(client->executable) || !cbm_validate_shell_arg(status_root)) {
10351041
return CBM_SVN_PROBE_UNCERTAIN;
10361042
}
1037-
#endif
10381043
char command[CBM_SZ_16K];
10391044
int written = snprintf(command, sizeof(command),
10401045
"\"%s\" status --xml --verbose --no-ignore --depth infinity "
@@ -1043,8 +1048,8 @@ cbm_svn_probe_result_t cbm_svn_probe(const cbm_svn_client_t *client, const char
10431048
if (written < 0 || (size_t)written >= sizeof(command)) {
10441049
return CBM_SVN_PROBE_UNCERTAIN;
10451050
}
1046-
10471051
FILE *process = cbm_popen(command, "r");
1052+
#endif
10481053
if (!process) {
10491054
return CBM_SVN_PROBE_UNCERTAIN;
10501055
}

tests/test_main.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,20 @@ static int tf_maybe_run_socket_probe(int argc, char **argv) {
148148
#endif
149149
}
150150

151+
static int tf_maybe_run_argv_probe(int argc, char **argv) {
152+
#ifdef _WIN32
153+
if (argc < 4 || strcmp(argv[1], "__cbm_argvprobe") != 0) {
154+
return -1;
155+
}
156+
puts(argv[2]);
157+
return atoi(argv[3]);
158+
#else
159+
(void)argc;
160+
(void)argv;
161+
return -1;
162+
#endif
163+
}
164+
151165
static int g_suite_argc = 0;
152166
static char **g_suite_argv = NULL;
153167
static bool *g_suite_arg_matched = NULL;
@@ -284,6 +298,10 @@ extern void suite_dump_verify_io(void);
284298
extern void cbm_kind_in_set_free_cache(void);
285299

286300
int main(int argc, char **argv) {
301+
int argv_probe_rc = tf_maybe_run_argv_probe(argc, argv);
302+
if (argv_probe_rc >= 0) {
303+
return argv_probe_rc;
304+
}
287305
/* #798 follow-up: if spawned as the socket-isolation probe, report whether an
288306
* inheritable socket handle crossed into this child and exit before any suite. */
289307
int probe_rc = tf_maybe_run_socket_probe(argc, argv);

tests/test_security.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,22 @@ TEST(popen_isolated_propagates_exit_code) {
707707
PASS();
708708
}
709709

710+
TEST(popen_argv_direct_spawn_propagates_exit_code) {
711+
char self[MAX_PATH];
712+
ASSERT(GetModuleFileNameA(NULL, self, sizeof(self)) > 0);
713+
const char *literal = "%CBM_POPEN_ARGV_LITERAL% & value";
714+
const char *argv[] = {self, "__cbm_argvprobe", literal, "37", NULL};
715+
FILE *fp = cbm_popen_argv(argv);
716+
ASSERT_NOT_NULL(fp);
717+
ASSERT_EQ(cbm_popen_last_was_isolated(), 1);
718+
char line[128];
719+
ASSERT_NOT_NULL(fgets(line, sizeof(line), fp));
720+
line[strcspn(line, "\r\n")] = '\0';
721+
ASSERT_STR_EQ(line, literal);
722+
ASSERT_EQ(cbm_pclose(fp), 37);
723+
PASS();
724+
}
725+
710726
/* #798 follow-up (the full-repro gap flagged above): prove the EXACT handle class
711727
* that deadlocked git — an inheritable AFD/listening-socket handle, the kind the
712728
* UI HTTP server holds — does NOT cross into the cbm_popen child. Unlike the
@@ -837,6 +853,7 @@ SUITE(security) {
837853
/* Isolated popen — handle-inheritance regression guard for #798 */
838854
RUN_TEST(popen_isolated_git_version_round_trip);
839855
RUN_TEST(popen_isolated_propagates_exit_code);
856+
RUN_TEST(popen_argv_direct_spawn_propagates_exit_code);
840857
RUN_TEST(popen_isolates_listening_socket);
841858
#endif
842859
}

tests/test_svn_state.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,9 @@ TEST(path_arg_matches_client_runtime) {
203203
char path[128];
204204
#ifdef _WIN32
205205
client.uses_posix_paths = true;
206-
ASSERT_TRUE(cbm_svn_format_path_arg(&client, "D:\\workspace\\project", path, sizeof(path)));
207-
ASSERT_STR_EQ(path, "/d/workspace/project");
206+
ASSERT_TRUE(
207+
cbm_svn_format_path_arg(&client, "D:\\R&D\\%project%", path, sizeof(path)));
208+
ASSERT_STR_EQ(path, "/d/R&D/%project%");
208209
#else
209210
ASSERT_TRUE(cbm_svn_format_path_arg(&client, "/workspace/project", path, sizeof(path)));
210211
ASSERT_STR_EQ(path, "/workspace/project");
@@ -220,8 +221,12 @@ TEST(client_runtime_detection_is_case_insensitive) {
220221

221222
TEST(real_probe_tracks_repeated_working_copy_edits) {
222223
th_svn_fixture_t fixture;
223-
ASSERT_EQ(th_svn_fixture_init(&fixture, "cbm_svn_probe", "source.c", "int value = 1;\n"),
224-
0);
224+
#ifdef _WIN32
225+
const char *prefix = "cbm_svn_probe_%_and&";
226+
#else
227+
const char *prefix = "cbm_svn_probe";
228+
#endif
229+
ASSERT_EQ(th_svn_fixture_init(&fixture, prefix, "source.c", "int value = 1;\n"), 0);
225230
cbm_svn_observation_t non_working_copy = {0};
226231
ASSERT_EQ(cbm_svn_probe(&fixture.client, fixture.root, &non_working_copy),
227232
CBM_SVN_PROBE_NOT_WORKING_COPY);

0 commit comments

Comments
 (0)