fix(worker): restore getpwnam execuser-existence check (bug-1507 merge regression)

The scheduler-framework merge (4e894d21) reverted the bug-1507 fix from
c091a8da: job_start_local_task fell back to nobody only when execuser was
EMPTY, not when it does not exist on the worker host — a submit-user missing
from /etc/passwd makes systemd fail the worker unit with EXIT_USER (217) and
every job dies. Restored the getpwnam() existence check (+ #include <pwd.h>).
All 21 worker tests pass; fallback log message verified in the built xcecd.

Also records the merge-overwrite lesson in cerebrum Do-Not-Repeat: after any
branch-sync merge, re-verify each previously-fixed bug's marker symbol
survived (git show <merge>:<file> | grep -c <symbol> vs <merge>^).
This commit is contained in:
lei.chen 2026-08-27 13:11:09 +00:00
parent 10865a4e63
commit 05ac57b183
4 changed files with 26 additions and 5 deletions

View file

@ -14867,20 +14867,22 @@
"error_message": "\u4f5c\u4e1a\u63d0\u4ea4\u540e\u7acb\u5373 EXIT: systemd \u5355\u5143 xcecd-worker-<jobid>.service \u5931\u8d25, status=217/USER (\u670d\u52a1\u7528\u6237\u4e0d\u5b58\u5728). \u7528\u6237 1-0-1785897786-0 \u7b49\u5168\u90e8\u5931\u8d25.",
"file": "src/worker/src/controller_connector.c (job_start_local_task)",
"root_cause": "apiserver bug-1500 \u4fee\u590d\u8bbe\u7f6e si.execuser=resolveExecUser(job)=\"admin\" (\u63d0\u4ea4\u7528\u6237). \u4f46 admin \u5728 worker \u4e3b\u673a /etc/passwd \u4e0d\u5b58\u5728. worker \u628a limits->user_name \u8bbe\u4e3a execuser, start_systemd_job \u751f\u6210 User=admin \u5355\u5143 -> systemd EXIT_USER 217 -> \u4f5c\u4e1a\u5931\u8d25. \u4e4b\u524d execuser \u4e3a\u7a7a\u65f6\u9ed8\u8ba4 nobody(\u5b58\u5728) \u80fd\u8dd1, bug-1500 \u90e8\u7f72\u540e\u53d8\u6210 admin \u53cd\u800c\u5168\u5931\u8d25.",
"fix": "job_start_local_task \u7528 getpwnam() \u68c0\u67e5 execuser \u662f\u5426\u5b58\u5728; \u4e0d\u5b58\u5728\u5219\u56de\u9000 nobody. \u4f5c\u4e1a\u4ee5 nobody \u8fd0\u884c(\u7528\u6237\u5df2\u786e\u8ba4\u73b0\u9636\u6bb5\u53ef\u4ee5).",
"fix": "job_start_local_task \u7528 getpwnam() \u68c0\u67e5 execuser \u662f\u5426\u5b58\u5728; \u4e0d\u5b58\u5728\u5219\u56de\u9000 nobody. \u4f5c\u4e1a\u4ee5 nobody \u8fd0\u884c(\u7528\u6237\u5df2\u786e\u8ba4\u73b0\u9636\u6bb5\u53ef\u4ee5). [2026-08-26 UPDATE] MERGE REGRESSION: the getpwnam() existence check was reverted by merge commit 4e894d21 (Rongsong scheduler-framework sync) \u2014 controller_connector.c now falls back to nobody only when execuser is empty, not when it does not exist (getpwnam gone from whole tree, verified 0 hits; chatbot^ had 1). admin exists on t1 /etc/passwd so not currently triggering, but latent \u2014 restore the getpwnam check (c091a8da had it). [2026-08-26] RESTORED: re-applied getpwnam() check in job_start_local_task (+ #include <pwd.h>), aligned to c091a8da. Worker builds clean, all 21 worker tests pass (controller_connector_test 15/0, resource_sync 2/0, xcecd_startup 2/0, dbus_workers 2/0). Verified the fallback log message is in the built xcecd binary.",
"tags": [
"worker",
"execuser",
"systemd",
"217",
"nobody",
"regression"
"regression",
"merge-regression",
"needs-restore"
],
"related_bugs": [
"bug-1500"
],
"occurrences": 1,
"last_seen": "2026-08-05"
"occurrences": 2,
"last_seen": "2026-08-26T12:00:00Z"
},
{
"id": "bug-1508",

View file

@ -118,6 +118,7 @@
## Do-Not-Repeat
- (2026-08-26, scheduler-framework merge) **A git merge that syncs another developer's branch CAN silently REVERT our already-committed fixes** when the other branch has an older/different version of the same file — the file-level resolution picks the other side, and our fix is gone with no conflict marker. This session found TWO such regressions from `4e894d21` (Rongsong scheduler-framework sync): bug-1490 (clib.go `ControlJob` lost `jobCtrlDiscriminant()`, so every kill/stop/resume failed `-EINVAL`) and bug-1507 (controller_connector.c lost the `getpwnam()` execuser-existence check, so a submit-user missing from the worker host's /etc/passwd kills every job with systemd 217). **Post-merge checklist — do this on EVERY branch-sync merge before committing/running tests:** (1) enumerate our files/fixes that the other branch also touches (diff our tip vs theirs: `git log --all --oneline -S <symbol>` for each fix's signature symbol, or `git diff <their-branch>...<our-branch> -- <files>`); (2) for each fix's marker symbol, assert it survived the merge: `git show <merge>:<file> | grep -c <symbol>` vs `git show <merge>^:<file> | grep -c <symbol>` — 0-after-1-before means the merge reverted it; (3) look specifically for defensive/fallback branches (`getpwnam` fallback, union-discriminant mapping, null-guards) — exactly the kind of safety code a sync commonly clobbers. For xce: our tracked worker/controller fixes are listed in `.wolf/buglog.json` tags `fixed` — after any future merge, re-verify each one's marker symbol is still present.
- (2026-07-29, bug-1482) When a code-review finding says "X is never freed, add a free() call" citing a `_for_test` wrapper's own doc comment as the ownership contract, VERIFY the doc comment against the actual callee before complying literally — don't just trust it. `myreq_handler_for_test`'s comment claimed "the caller still owns/frees `pkt` itself", but `myreq_handler` unconditionally calls `xce_decode(pkt, ...)` as its first step, and `xce_decode()` (src/lib/request.c:274-275) itself unconditionally does `free_packet(pkt); FREEUP(pkt);` before returning — pkt is fully consumed/freed internally, no leak exists. Blindly adding the suggested `free_packet(pkt); free(pkt);` after the call produced an immediate ASAN heap-use-after-free/double-free (independent of `detect_leaks`, since UAF detection isn't gated by leak detection) — caught only by actually rebuilding and running the authoritative test command, not by code inspection alone. Fix was to correct the misleading doc comment instead of adding the free. **Lesson: for any wrapper that decodes/consumes a buffer (xce_decode-style), trace the actual free call sites transitively before trusting an ownership comment — a stale/wrong doc comment is itself a bug, and "add the missing free" is not always the right fix for a reported leak.**
- (2026-07-19, bug-1452) `commit_engine_fini(&ce)` MUST run AFTER `persist_stop(p)`, never before, whenever the `persist_t` was `persist_start`'d (i.e. has a live `fsync_worker` thread). `commit_engine_fini`'s own header comment says this explicitly ("Callers are expected to have stopped the flusher (persist_stop) first") but `test_persist_truncate.c`'s three continuation tests called it backwards (`commit_engine_fini` then `teardown_persist`→`persist_stop`). `persist_set_durable_cb(p, NULL, NULL)` inside `commit_engine_fini` is an unsynchronized plain-pointer unhook — while the flusher thread is still alive it can already be mid-flight in `commit_on_durable_advance`/`commit_advance_locked` (no lock held, per persist.c:398-399's documented lock-ordering) and race the teardown: the guard `ce->p == NULL` check passes on a still-valid read, then a SEPARATE later re-read of `ce->p` inside `commit_advance_locked` (commit.c:208) observes the value `commit_engine_fini` just nulled — SEGV at exactly `offsetof(persist_t, committed_lsn)` (0x38). This was misdiagnosed for a day as caused by the g_drive_async_demotion/catchup_offload flags (bug-1450/1451) because flipping them changed thread-scheduling timing enough to trigger the always-latent race more reliably — the SAME crash signature was independently seen on 2026-07-07, before those flags existed, proving the flags were never causal. Fix: call `persist_stop(p)` immediately before `commit_engine_fini(&ce)`; the teardown helper's own internal `persist_stop` becomes a safe no-op second call. **Same backwards ordering also exists in `test_demotion_convergence.c` (line 233 `commit_engine_fini` before line 235 `persist_stop`) against a real `persist_start`'d fixture — not yet fixed, flag for a follow-up sweep of every `commit_engine_fini` call site in the controller test suite.**
- (2026-07-03, raft-apply-decoupling task 1.5) `ninja -C build`'s INCREMENTAL rebuild in this worktree's shared `build/` dir can silently produce a stale/wrong `controller_unit_test` binary that reports a build success but gives WRONG test results for the actual HEAD source — distinct from bug-924 (which is the `meson test` driver hanging on overlayfs fdatasync). Symptom that tipped this off: `test_config_repl`'s `test_commit_advance_retry_clears_push_before_commit_fence` was independently measured as 70/70-passing by one task's reviewer and 10/10-failing by the next task's implementer, on what both believed was the same commit. Root-caused by isolating each measurement in its own throwaway `git worktree add <tmp-path> <commit>` + fresh `meson setup --buildtype=debug -Db_sanitize=address` + `ninja -C <tmp-build>` — proved the SAME commit passes cleanly with a fresh build and fails deterministically only in the main worktree's incrementally-rebuilt `build/`. Fix was `rm -rf build && meson setup build ... && ninja -C build` in the main worktree. **Lesson: whenever a test result looks surprising or contradicts a very recent measurement on the same commit, do NOT immediately conclude a source regression — first rule out a stale incremental build by either a full `rm -rf build` rebuild or an isolated `git worktree add` + fresh setup.** Never mutate the current worktree in place to "check an old commit" (e.g. `git checkout <sha> -- .`) — it silently reverts working-tree files against current HEAD; use `git worktree add` instead.

View file

@ -7406,3 +7406,4 @@
| 15:5x | controller full suite 3 FAILs root-caused: Rongsong WAL_JOB_DISPATCH replay tests assert JOB_STAT_RUN after persist_recover, but our bug-1509 B3 (mark_all_run_jobs_unknown_in_memory in persist_recover, p->cluster set) ORs JOB_STAT_UNKWN|TRANSIENT onto RUN. Reconciled by updating the 3 tests (test_wal_job_dispatch_fact.c x2, test_dispatch_commit.c x1) to assert JOB_IS_RUN (base RUN recovered, Rongsong's intent incl. bug-1678 allocs) + UNKWN flag set (our B3). Both groups pass. | controller/testsuite/*.c | 6+4 tests OK | ~2k |
| 10:3x | merged 集群重启+验证:nix build '.#xce' 产出 w96pv5-xcecore(含 pending_after 修复);AppRun/runtime 被 nix-store --gc 清掉 → package.sh 用旧 AppImage 提取的 AppRun + 拆出的 runtime ELF 前缀打包,但 `find(b'hsqs')` 命中了 runtime 内部数据 → runtime 被截断(22460B),新 AppImage --appimage-extract 崩 SIGILL。绕过:unsquashfs -d 解包 squashfs 到 /opt/xce/appimage/squashfs-root + touch marker 让 start-cluster.sh 跳过 --appimage-extract(t1 用真实 nix store w96pv5)。集群起来:jobs submit(含 from_host)→RUN→DONE→kill 全通,hosts max_slot=8/NUM_RUN/HOST_TYPE 正常。**新发现 merge 回归**:Rongsong clib.go 覆盖了我们的 bug-1490 修复(ControlJob 的 ctrl.action 应映射到 capnp JobControl union 判别值 0/1,不是 Go JOB_* 常量 1/2/3/8)→ 所有 kill/stop 都 -22 EINVAL。已恢复 `ctrl.action = C.int(jobCtrlDiscriminant(action))`(helper 在 convert_pure.go:30 还活着)+ 重建 apiserver 到 /tmp/apiserver_merged2 + 重启 apiserver,kill 验证通过。待办:重建 xce(含 clib.go 修复)+打包 AppImage 做干净部署;修 AppImage runtime(从 nixpkgs 重建 AppRun+runtime)。 | persist.c, clib.go, start-cluster.sh, package.sh | 集群 OK;bug-1490 回归已修 | ~4k |
| 11:5x | 遗留项处理完成:(1) 重建 xce 含 clib.go 修复 → 新 xcecore zjxxy909,apiserver 已从 store 干净 build 重启(非 /tmp 补丁),submit/kill/hosts 验证通过。(2) AppImage runtime 修复:nixpkgs 无 classic appimageTools(都被 GC/新版风格),下载 appimagetool(github continuous, 8.8MB)→ APPIMAGE_EXTRACT_AND_RUN=1 绕过 FUSE → 从 AppDir(AppRun+mountroot+99 store paths 闭包+desktop+icon)构建出正常 AppImage(--appimage-extract 可用)。注意:nix copy --to file:// 会建 binary cache 布局不是 nix/store 布局;cp -al 对目录 hardlink 失败、cp -a 保留只读权限难清理,用 cp -r 重建 AppDir。 | AppImage zjxxy, apiserver store build | 全绿 | ~3k |
| 12:1x | bug-1507 merge 回归修复:恢复 getpwnam() execuser 存在性检查(+pwd.h),对齐 c091a8da。worker 全测试 21/0 通过。cerebrum 加 Do-Not-Repeat:merge 同步会静默覆盖我们的修复(bug-1490 clib.go + bug-1507 getpwnam 两个实例)——post-merge 必须核对每个 fixed bug 的 marker symbol 是否幸存(git show <merge>:<file>|grep -c vs <merge>^)。 | controller_connector.c, cerebrum.md | 修复+教训记录 | ~500 |

View file

@ -8,6 +8,7 @@
#include "xcelog.h"
#include <errno.h>
#include <fcntl.h>
#include <pwd.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
@ -1000,7 +1001,23 @@ static int job_start_local_task(xce_request_t* request)
}
if (job_ctx->startinfo->execuser != NULL) {
job_ctx->limits->user_name = strdup(job_ctx->startinfo->execuser);
/* bug-1507 (merge-regression-restored): the exec user is the
* submitting account, which is not necessarily a real /etc/passwd
* account on this worker host, and systemd then fails the worker
* unit with EXIT_USER (217), killing every job. Fall back to
* nobody so the job still runs. (Restored from c091a8da — the
* scheduler-framework merge 4e894d21 had reverted this check.) */
struct passwd* pw = getpwnam(job_ctx->startinfo->execuser);
if (pw != NULL) {
job_ctx->limits->user_name
= strdup(job_ctx->startinfo->execuser);
} else {
job_ctx->limits->user_name = strdup("nobody");
XCECD_LOG(XCE_LOG_WARN,
"exec user '%s' does not exist on this host, "
"running job as nobody",
job_ctx->startinfo->execuser);
}
} else {
job_ctx->limits->user_name = strdup("nobody");
XCECD_LOG(XCE_LOG_WARN,