github-featured/deusdata--codebase-memory-mcp
2
0
Fork 0
Code Issues Pull Requests Deployments Models Agent Notes Packages Projects Releases Wiki Activity

fix(test-infra): grant the build-dir ACL by SID, not by name (#1532 follow-up)

Browse Source 
The local Windows leg caught this within hours of merging #1532: on a machine
that is not domain-joined, USERDOMAIN is the literal string WORKGROUP, and
`WORKGROUP\test` is not a resolvable principal —

    WARN: build-dir DACL stamp (pre-wave) failed (user=WORKGROUP\test ...):
    WORKGROUP\test: No mapping between account names and security IDs was done.

so the grant fails outright and the tree keeps its inherited
`Authenticated Users:(M)`. The guard then correctly reports that cross-account
mutation is still possible, and the whole leg stops. Since most Windows
machines are not domain-joined, that is the common case, not the exotic one.

#1532 was fixing a real bug in the other direction (a bare name resolves
against the machine first, so a host named like its user grants to an empty
principal). Both failures are name RESOLUTION failures, so this stops resolving
names: the account is identified by SID, which is unambiguous everywhere. The
SYSTEM and Administrators grants in these same commands have always used that
form — the current user was the odd one out. Name lookup survives only as a
fallback where PowerShell is unavailable.

Applied to all three scripts carrying the stamp (run-tests-parallel, soak-test,
memlab), since they share the failure and would otherwise drift apart.

Co-Authored-By: Kiborgik <Kiborgik@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-08-12 11:57:35 +02:00
parent 8d487b7da0
commit a654acbc88
5 changed files with 128 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/memlab.sh
+21 -4
View File
@@ -38,10 +38,27 @@ if [[ "$BINARY" == *.exe ]] && command -v cygpath >/dev/null 2>&1 &&
# Qualify with the domain: a bare name from coreutils `whoami` resolves
# against the machine first, so on a host whose name equals the user's the
# grant lands on an empty principal.
ME="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
ME="${USERDOMAIN}\\${ME}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
ME="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${ME}" in
S-1-*) ME="*${ME}" ;;
*)
ME="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
ME="${USERDOMAIN}\\${ME}"
fi
;;
esac
MSYS2_ARG_CONV_EXCL='*' icacls "$WIN_ROOT_W" /reset /Q >/dev/null 2>&1 || true
if ! MSYS2_ARG_CONV_EXCL='*' icacls "$WIN_ROOT_W" /inheritance:r \
/grant:r "${ME}:(OI)(CI)F" '*S-1-5-18:(OI)(CI)F' '*S-1-5-32-544:(OI)(CI)F' \
scripts/run-tests-parallel.sh
+21 -4
View File
@@ -64,10 +64,27 @@ stamp_windows_build_dir() {
# (COMPUTERNAME=BUILD, user build) the grant lands on an empty principal
# (BUILD\) and, combined with /inheritance:r above, locks this script out
# of its own log directory.
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
me="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${me}" in
S-1-*) me="*${me}" ;;
*)
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
;;
esac
# Normalize FIRST: some runner images stamp EXPLICIT (non-inherited)
# Authenticated-Users ACEs onto the workspace tree, which /inheritance:r
# cannot strip and /grant:r does not touch (it replaces only the granted
scripts/soak-test.sh
+21 -4
View File
@@ -80,10 +80,27 @@ soak_stamp_windows_dir() {
# Qualify with the domain: a bare name from coreutils `whoami` resolves
# against the machine first, so on a host whose name equals the user's the
# grant lands on an empty principal.
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
me="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${me}" in
S-1-*) me="*${me}" ;;
*)
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
;;
esac
if ! output=$(MSYS2_ARG_CONV_EXCL='*' icacls "$dir_w" /reset /Q 2>&1); then
echo "FAIL: soak DACL normalize failed (dir=$dir_w user=$me)" >&2
printf '%s\n' "$output" >&2
src/mcp/mcp.c
+9 -1
View File
@@ -601,7 +601,7 @@ static const tool_def_t TOOLS[] = {
"\"description\":\"Max enriched results per call. Default 10. Response includes "
"'total_grep_matches' and 'total_results' so callers can detect truncation. No "
"offset parameter — raise limit or narrow with file_pattern / path_filter to see more."
"\",\"default\":10}},\"required\":[\"pattern\",\"project\"]}"},
"\",\"default\":10,\"minimum\":1}},\"required\":[\"pattern\",\"project\"]}"},
{"list_projects", "List projects", "List all indexed projects",
"{\"type\":\"object\",\"properties\":{}}"},
@@ -9634,6 +9634,14 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
char *path_filter = cbm_mcp_get_string_arg(args, "path_filter");
char *mode_str = cbm_mcp_get_string_arg(args, "mode");
int limit = cbm_mcp_get_int_arg(args, "limit", MCP_DEFAULT_LIMIT);
/* #1511: a negative limit flowed straight into the result cap and came back
* as the reported count ("results: -5"), which reads to an agent as a real
* answer rather than a rejected argument. The schema now declares
* minimum:1, but a schema is a request to the client, never a guarantee to
* the server — clamp here too. */
if (limit < 1) {
limit = MCP_DEFAULT_LIMIT;
}
int context_lines = cbm_mcp_get_int_arg(args, "context", 0);
bool use_regex = cbm_mcp_get_bool_arg(args, "regex");
uint64_t search_t0 = cbm_now_ms();
tests/test_mcp.c
+56
View File
@@ -4012,6 +4012,60 @@ TEST(tool_search_code_missing_pattern) {
PASS();
}
/* #1511 (distilled from @lukiod's #1512): search_code echoed a negative limit
* back as the result count — "results: -5" — which an agent reads as an answer,
* not as a rejected argument. Both halves matter: the schema declares the bound
* so well-behaved clients never send it, and the handler clamps because a
* schema is a request to the client, never a guarantee to the server. */
TEST(tool_search_code_negative_limit_is_not_echoed_issue1511) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char *resp =
cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":35,\"method\":\"tools/call\","
"\"params\":{\"name\":\"search_code\","
"\"arguments\":{\"pattern\":\"func main\","
"\"project\":\"nonexistent\",\"limit\":-5}}}");
ASSERT_NOT_NULL(resp);
ASSERT_NULL(strstr(resp, "results: -5"));
free(resp);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_search_code_limit_declares_a_minimum_issue1511) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char *resp = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":36,\"method\":\"tools/list\",\"params\":{}}");
ASSERT_NOT_NULL(resp);
yyjson_doc *doc = yyjson_read(resp, strlen(resp), 0);
yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL;
yyjson_val *result = root ? yyjson_obj_get(root, "result") : NULL;
yyjson_val *tools = result ? yyjson_obj_get(result, "tools") : NULL;
yyjson_val *minimum = NULL;
if (tools && yyjson_is_arr(tools)) {
size_t index, max;
yyjson_val *tool;
yyjson_arr_foreach(tools, index, max, tool) {
yyjson_val *name = yyjson_obj_get(tool, "name");
if (!name || !yyjson_is_str(name) || strcmp(yyjson_get_str(name), "search_code") != 0) {
continue;
}
yyjson_val *schema = yyjson_obj_get(tool, "inputSchema");
yyjson_val *props = schema ? yyjson_obj_get(schema, "properties") : NULL;
yyjson_val *limit = props ? yyjson_obj_get(props, "limit") : NULL;
minimum = limit ? yyjson_obj_get(limit, "minimum") : NULL;
break;
}
}
bool declared = minimum && yyjson_is_int(minimum) && yyjson_get_int(minimum) >= 1;
yyjson_doc_free(doc);
free(resp);
cbm_mcp_server_free(srv);
ASSERT_TRUE(declared);
PASS();
}
TEST(tool_search_code_no_project) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
@@ -10591,6 +10645,8 @@ SUITE(mcp) {
RUN_TEST(tool_get_code_snippet_missing_qn);
RUN_TEST(tool_get_code_snippet_not_found);
RUN_TEST(tool_search_code_missing_pattern);
RUN_TEST(tool_search_code_negative_limit_is_not_echoed_issue1511);
RUN_TEST(tool_search_code_limit_declares_a_minimum_issue1511);
RUN_TEST(tool_search_code_no_project);
RUN_TEST(search_code_multi_word);
RUN_TEST(search_code_scoped_path_with_spaces_issue687);