fix(a3): resolve shift buffer race condition with call-unique tokens, atomic write, and automatic GC (100% PASS)
This commit is contained in:
+43
-7
@@ -450,13 +450,40 @@ except Exception:
|
||||
exit 1
|
||||
fi
|
||||
buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')"
|
||||
if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi
|
||||
if [ -z "$buf" ]; then
|
||||
echo "Error: -b name contains no usable characters" >&2
|
||||
exit 1
|
||||
fi
|
||||
buf="buf_$buf"
|
||||
shift 2
|
||||
;;
|
||||
*) text="$1"; shift ;;
|
||||
esac
|
||||
done
|
||||
echo -n "$text" > "$wrapper_dir/$buf"
|
||||
# A-3 GC: unique buffer names mean an abandoned buffer (SIGINT/SIGTERM/
|
||||
# timeout between set-buffer and delete-buffer) is never overwritten, so
|
||||
# it would leak forever. Sweep here -- at creation time, the moment new
|
||||
# garbage can appear -- rather than from a separate reaper.
|
||||
#
|
||||
# SAFETY: the age threshold must comfortably exceed the lifetime of a LIVE
|
||||
# buffer, or the sweep would delete a buffer awaiting its paste and
|
||||
# re-create the silent loss A-3 exists to remove. A live buffer lives from
|
||||
# set-buffer to delete-buffer (sub-second in practice, bounded by the
|
||||
# blocking `agent send`), so 60 minutes is ~4 orders of magnitude of slack.
|
||||
# Failures are ignored: garbage collection must never break an injection.
|
||||
#
|
||||
# The '.buf_sks_*' arm is not redundant: F2's atomic write stages through
|
||||
# a DOT-prefixed temp ('.buf_sks_....tmp'), which 'buf_sks_*' cannot match.
|
||||
# Without it a temp orphaned by SIGKILL between write and rename would leak
|
||||
# forever, exactly like the buffers this sweep exists to reclaim.
|
||||
find "$wrapper_dir" \( -name 'buf_sks_*' -o -name '.buf_sks_*' \) \
|
||||
-mmin +${MAM_BUFFER_GC_MINUTES:-60} -delete 2>/dev/null || true
|
||||
_tmp="$wrapper_dir/.$buf.$$.tmp"
|
||||
if ! { echo -n "$text" > "$_tmp" && mv -f "$_tmp" "$wrapper_dir/$buf"; }; then
|
||||
rm -f "$_tmp"
|
||||
echo "Error: failed to write buffer $buf" >&2
|
||||
exit 1
|
||||
fi
|
||||
;;
|
||||
paste-buffer)
|
||||
buf="tmp_buffer"
|
||||
@@ -465,7 +492,11 @@ except Exception:
|
||||
case "$1" in
|
||||
-b)
|
||||
buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')"
|
||||
if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi
|
||||
if [ -z "$buf" ]; then
|
||||
echo "Error: -b name contains no usable characters" >&2
|
||||
exit 1
|
||||
fi
|
||||
buf="buf_$buf"
|
||||
shift 2
|
||||
;;
|
||||
-t)
|
||||
@@ -492,7 +523,11 @@ except Exception:
|
||||
case "$1" in
|
||||
-b)
|
||||
buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')"
|
||||
if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi
|
||||
if [ -z "$buf" ]; then
|
||||
echo "Error: -b name contains no usable characters" >&2
|
||||
exit 1
|
||||
fi
|
||||
buf="buf_$buf"
|
||||
shift 2
|
||||
;;
|
||||
*) shift ;;
|
||||
@@ -1706,9 +1741,10 @@ send_keys_safe() {
|
||||
sleep 2
|
||||
done
|
||||
|
||||
_sks_herdr set-buffer -b "sks_$job_id" "$text"
|
||||
_sks_herdr paste-buffer -b "sks_$job_id" -t "$sess"
|
||||
_sks_herdr delete-buffer -b "sks_$job_id" 2>/dev/null || true
|
||||
local sks_buf="sks_${sess}_${job_id}_$$_${RANDOM}_$(date +%s%N 2>/dev/null || date +%s)"
|
||||
_sks_herdr set-buffer -b "$sks_buf" "$text"
|
||||
_sks_herdr paste-buffer -b "$sks_buf" -t "$sess"
|
||||
_sks_herdr delete-buffer -b "$sks_buf" 2>/dev/null || true
|
||||
if [[ "$sess" =~ "agy" ]]; then
|
||||
_sks_herdr send-keys -t "$sess" C-m
|
||||
return 0
|
||||
|
||||
+11
-9
@@ -1,9 +1,9 @@
|
||||
# 🛠️ Multi-Agent Mux 종합 개선 및 미해결 과제 백로그 (`IMPROVEMENTS.md`)
|
||||
|
||||
- **최종 갱신일**: 2026-08-06 (B-3 herdr real binary preflight verification 완료 반영)
|
||||
- **최종 갱신일**: 2026-08-06 (A-3 call-unique buffer isolation & automatic GC 완료 반영)
|
||||
- **통합 관리 대상**: 기존 `CODEBASE_REVIEW_REPORT.md` + `OPTIMIZATION.md`
|
||||
- **총 추적 미해결 과제**: **15건** (아키텍처 2건, 엣지케이스 7건, 오케스트레이션 2건, 레거시 잔재 4건)
|
||||
- **완료된 과제**: **6건** (A-1, A-5, B-1, B-3, C-1, O-1)
|
||||
- **총 추적 미해결 과제**: **14건** (아키텍처 1건, 엣지케이스 7건, 오케스트레이션 2건, 레거시 잔재 4건)
|
||||
- **완료된 과제**: **7건** (A-1, A-3, A-5, B-1, B-3, C-1, O-1)
|
||||
|
||||
---
|
||||
|
||||
@@ -13,16 +13,12 @@
|
||||
|
||||
---
|
||||
|
||||
## 1. 🔴 아키텍처 결함 (Architecture Flaws — 2건)
|
||||
## 1. 🔴 아키텍처 결함 (Architecture Flaws — 1건)
|
||||
|
||||
### **A-2: 공개 브로커 + HMAC 인증 Off + 와일드카드 전파**
|
||||
- **현상**: `mqtt_common.py`의 기본 브로커가 공개 서버(`broker.hivemq.com`), HMAC 무조건 True 반환으로 설정되어 있습니다.
|
||||
- **파급 효과**: 외부에서 유입되는 malicious `error` 이벤트 수신 시 `reconcile.sh`가 라이브 에이전트 pane을 `kill-session`으로 강제 파괴하는 치명적 보안/안정성 위험이 존재합니다.
|
||||
|
||||
### **A-3: 시프트 버퍼 단일 파일 공유 및 동시 주입 오염**
|
||||
- **현상**: `send_keys_safe` 시프트의 `set/paste/delete-buffer`가 `-b` 세션 버퍼 이름을 무시하고 단일 `.mam/shim/tmp_buffer` 파일 하나만을 공유합니다.
|
||||
- **파급 효과**: 다중 에이전트 동시 주입 시 대화 텍스트 교차 오염 및 무음 유실(t5 silent loss)이 발생합니다.
|
||||
|
||||
---
|
||||
|
||||
## 2. 🟠 엣지 케이스 및 런타임 버그 (Edge-case Bugs — 7건)
|
||||
@@ -84,12 +80,18 @@
|
||||
|
||||
---
|
||||
|
||||
## 5. 🎉 완료된 과제 (Completed Tasks — 6건)
|
||||
## 5. 🎉 완료된 과제 (Completed Tasks — 7건)
|
||||
|
||||
### **A-1: 워크스페이스 세션 격리 & drift-B 오등록 방지** — ✅ 완료
|
||||
- `derive_workspace_slug` 헬퍼 함수를 추가하여 워크스페이스 경로 기반 단일 소켓 슬러그(`mam-<parent>-<work>`) 도출 체계를 구축했습니다.
|
||||
- `reconcile.sh` drift-B 자동 등록 시 foreign cwd 차단 게이트를 구축하여 세션 오등록을 방지했습니다.
|
||||
|
||||
### **A-3: 시프트 버퍼 동시 주입 오염 & 자동 GC 체계 구축** — ✅ 완료
|
||||
- `lib.sh` 내 `send_keys_safe` 시프트 버퍼 명령의 임시 파일명을 `sks_${sess}_${job_id}_$$_${RANDOM}_$(date +%s%N)` 식 호출 단위 독립 토큰으로 변환하여 다중 에이전트 동시 주입 시 대화 교차 오염 및 무음 유실(T5)을 원천 차단했습니다.
|
||||
- `set-buffer` 시 원자적 임시 쓰기(`.$buf.$$.tmp`) 및 rename(`mv -f`) 구조를 구현하여 파티셜 레코드 관측을 방지했습니다.
|
||||
- 인터럽트/예외 종료 시 남는 stale 버퍼 파일을 자동으로 정리하는 60분 내장 GC(`find -mmin +60 -delete`)를 내장했습니다.
|
||||
- 전용 회귀 테스트 스위트 `tests/test_a3_buffer_isolation.py` (12/12 PASS)를 작성하여 입증했습니다.
|
||||
|
||||
### **A-5: `HERDR_SESSION_NAME` 네이티브 전환** — ✅ 완료
|
||||
- 기존 `HERDR_SERVER_NAME` 환경변수를 herdr 시프트가 직접 읽는 네이티브 **`HERDR_SESSION_NAME`** 및 YAML 레지스트리 키 **`herdr_session`**으로 전수 전환 단일화했습니다.
|
||||
|
||||
|
||||
@@ -0,0 +1,276 @@
|
||||
"""A-3 — shift buffer isolation & garbage collection.
|
||||
|
||||
Defects addressed:
|
||||
① Colliding buffer names (e.g. constant "sks_adhoc" or "sks_onboard") caused
|
||||
cross-agent prompt contamination and silent buffer deletion loss.
|
||||
② Call-unique buffer names (sks_<job>_<pid>_<rand>) fixed ① but introduced a
|
||||
permanent leak of abandoned buffers (SIGINT/SIGTERM/timeout between
|
||||
set-buffer and delete-buffer). Fixed with automatic GC (-mmin +60).
|
||||
③ F2's atomic write created DOT-prefixed temp files ('.buf_sks_....tmp') that
|
||||
F4's initial GC pattern ('buf_sks_*') missed. Fixed by widening the GC
|
||||
pattern to include '.buf_sks_*' and cleaning up on write/rename failure.
|
||||
"""
|
||||
import os
|
||||
import subprocess
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
BARE_PATH = "/usr/bin:/bin:/usr/sbin:/sbin"
|
||||
|
||||
|
||||
def _bash(sandbox, snippet, extra_env=None):
|
||||
env = dict(os.environ)
|
||||
env["PATH"] = BARE_PATH
|
||||
env["WORKSPACE_ROOT"] = str(sandbox)
|
||||
if extra_env:
|
||||
env.update(extra_env)
|
||||
script = f'source "{sandbox}/.agents/skills/lib.sh" >/dev/null 2>&1\n{snippet}'
|
||||
return subprocess.run(["bash", "-c", script], capture_output=True,
|
||||
text=True, cwd=str(sandbox), env=env)
|
||||
|
||||
|
||||
# Harness to invoke send_keys_safe without a running herdr daemon or tmux
|
||||
def _run_send_keys_safe(sandbox, sess, text, job_id=None):
|
||||
buflog = sandbox / "buflog.txt"
|
||||
job_arg = f' "{job_id}"' if job_id is not None else ""
|
||||
snippet = f"""
|
||||
_pane_quiescent() {{ return 0; }}
|
||||
_pane_dialog_open() {{ return 1; }}
|
||||
_pane_capture() {{ printf '%s' "$SKS_FAKE_PANE"; }}
|
||||
_sks_herdr() {{
|
||||
case "$1" in
|
||||
set-buffer|paste-buffer|delete-buffer)
|
||||
[ "$2" = "-b" ] && printf '%s %s\\n' "$1" "$3" >> "{buflog}" ;;
|
||||
*)
|
||||
return 0 ;;
|
||||
esac
|
||||
return 0
|
||||
}}
|
||||
export SKS_FAKE_PANE="● $text"
|
||||
send_keys_safe "{sess}" "{text}"{job_arg}
|
||||
"""
|
||||
res = _bash(sandbox, snippet)
|
||||
log_lines = buflog.read_text().splitlines() if buflog.exists() else []
|
||||
return res, log_lines
|
||||
|
||||
|
||||
# X-1 — Two calls with the same job_id (e.g. "onboard") get DIFFERENT buffer names
|
||||
def test_a3_same_job_id_gets_different_buffer_names(mam_sandbox):
|
||||
res1, lines1 = _run_send_keys_safe(mam_sandbox, "claude_sess1", "hello", "onboard")
|
||||
res2, lines2 = _run_send_keys_safe(mam_sandbox, "claude_sess2", "world", "onboard")
|
||||
|
||||
assert res1.returncode == 0 and res2.returncode == 0
|
||||
buf1 = lines1[0].split()[1]
|
||||
buf2 = lines2[-3].split()[1]
|
||||
assert buf1 != buf2, f"buffer names collided: {buf1} == {buf2}"
|
||||
|
||||
|
||||
# X-2 — Within one call, set-buffer, paste-buffer, and delete-buffer use the SAME name
|
||||
def test_a3_triple_uses_same_buffer_name(mam_sandbox):
|
||||
res, lines = _run_send_keys_safe(mam_sandbox, "claude_sess1", "test payload", "job1")
|
||||
assert res.returncode == 0
|
||||
assert len(lines) == 3
|
||||
cmd0, name0 = lines[0].split()
|
||||
cmd1, name1 = lines[1].split()
|
||||
cmd2, name2 = lines[2].split()
|
||||
|
||||
assert cmd0 == "set-buffer" and cmd1 == "paste-buffer" and cmd2 == "delete-buffer"
|
||||
assert name0 == name1 == name2
|
||||
|
||||
|
||||
# X-3 — Different buffer names map to different files; delete does not touch another's file
|
||||
def test_a3_different_names_do_not_interfere(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
SHIM="$WORKSPACE_ROOT/.mam/shim/herdr"
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
"$SHIM" set-buffer -b "buf_A" "PAYLOAD_A"
|
||||
"$SHIM" set-buffer -b "buf_B" "PAYLOAD_B"
|
||||
|
||||
assert_file() { [ -f "$DIR/$1" ] && echo "$1:EXISTS" || echo "$1:MISSING"; }
|
||||
assert_file "buf_buf_A"
|
||||
assert_file "buf_buf_B"
|
||||
|
||||
"$SHIM" delete-buffer -b "buf_A"
|
||||
assert_file "buf_buf_A"
|
||||
assert_file "buf_buf_B"
|
||||
""")
|
||||
assert "buf_buf_A:EXISTS" in res.stdout
|
||||
assert "buf_buf_B:EXISTS" in res.stdout
|
||||
assert "buf_buf_A:MISSING" in res.stdout
|
||||
assert "buf_buf_B:EXISTS" in res.stdout
|
||||
|
||||
|
||||
# X-4 — Invalid/empty -b names are rejected explicitly, not falling back to tmp_buffer
|
||||
def test_a3_unusable_buffer_name_is_rejected(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
SHIM="$WORKSPACE_ROOT/.mam/shim/herdr"
|
||||
"$SHIM" set-buffer -b "!!!" "PAYLOAD" 2>&1
|
||||
echo "RC:$?"
|
||||
""")
|
||||
assert "RC:1" in res.stdout
|
||||
assert "contains no usable characters" in res.stdout + res.stderr
|
||||
|
||||
|
||||
# X-5 — Parallel execution test: 12 callers get isolated buffer text
|
||||
def test_a3_parallel_calls_are_isolated(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
SHIM="$WORKSPACE_ROOT/.mam/shim/herdr"
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
|
||||
for i in $(seq 1 12); do
|
||||
"$SHIM" set-buffer -b "par_$i" "DATA_$i" &
|
||||
done
|
||||
wait
|
||||
|
||||
for i in $(seq 1 12); do
|
||||
content=$(cat "$DIR/buf_par_$i" 2>/dev/null)
|
||||
if [ "$content" != "DATA_$i" ]; then
|
||||
echo "CORRUPTION in par_$i: got '$content'"
|
||||
fi
|
||||
done
|
||||
echo "PARALLEL:OK"
|
||||
""")
|
||||
assert "PARALLEL:OK" in res.stdout
|
||||
assert "CORRUPTION" not in res.stdout
|
||||
|
||||
|
||||
# X-6 — Atomic write does not leave .tmp files behind on success
|
||||
def test_a3_atomic_write_leaves_no_tmp_on_success(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
SHIM="$WORKSPACE_ROOT/.mam/shim/herdr"
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
"$SHIM" set-buffer -b "atomic_test" "ATOMIC_PAYLOAD"
|
||||
ls -a "$DIR"
|
||||
""")
|
||||
assert "buf_atomic_test" in res.stdout
|
||||
assert not any(f.endswith(".tmp") for f in res.stdout.split())
|
||||
|
||||
|
||||
# X-7 — Abandoned buffers (older than GC threshold) are reclaimed
|
||||
def test_a3_abandoned_buffers_are_collected(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
ABANDONED="$DIR/buf_sks_OLD_1_1"
|
||||
: > "$ABANDONED"
|
||||
touch -t 202501010000 "$ABANDONED"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA"
|
||||
|
||||
if [ -f "$ABANDONED" ]; then
|
||||
echo "GC:LEAK"
|
||||
else
|
||||
echo "GC:SWEPT"
|
||||
fi
|
||||
""")
|
||||
assert "GC:SWEPT" in res.stdout
|
||||
|
||||
|
||||
# X-8 — Live buffers (newer than GC threshold) are preserved
|
||||
def test_a3_live_buffers_are_preserved(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
LIVE="$DIR/buf_sks_LIVE_2_2"
|
||||
echo "LIVE_CONTENT" > "$LIVE"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA"
|
||||
|
||||
if [ -f "$LIVE" ]; then
|
||||
echo "LIVE:PRESERVED"
|
||||
else
|
||||
echo "LIVE:DELETED"
|
||||
fi
|
||||
""")
|
||||
assert "LIVE:PRESERVED" in res.stdout
|
||||
|
||||
|
||||
# X-9 — Sweep is scoped to buf_sks_* and does not touch non-matching files
|
||||
def test_a3_sweep_is_scoped(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
OTHER="$DIR/other_old_file"
|
||||
: > "$OTHER"
|
||||
touch -t 202501010000 "$OTHER"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA"
|
||||
|
||||
if [ -f "$OTHER" ]; then
|
||||
echo "OTHER:PRESERVED"
|
||||
else
|
||||
echo "OTHER:DELETED"
|
||||
fi
|
||||
""")
|
||||
assert "OTHER:PRESERVED" in res.stdout
|
||||
|
||||
|
||||
# X-10 — Abandoned atomic write DOT-temp files (.buf_sks_*.tmp) are collected
|
||||
def test_a3_abandoned_dot_temps_are_collected(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
DOT_TEMP="$DIR/.buf_sks_ORPHAN_1_1.99.tmp"
|
||||
: > "$DOT_TEMP"
|
||||
touch -t 202501010000 "$DOT_TEMP"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA"
|
||||
|
||||
if [ -f "$DOT_TEMP" ]; then
|
||||
echo "DOT_TEMP:LEAK"
|
||||
else
|
||||
echo "DOT_TEMP:SWEPT"
|
||||
fi
|
||||
""")
|
||||
assert "DOT_TEMP:SWEPT" in res.stdout
|
||||
|
||||
|
||||
# X-11 — Failed rename cleans up temp file immediately and fails with exit code 1
|
||||
def test_a3_failed_rename_cleans_up_and_fails(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
# Block creation of buf_sks_blocked by placing a read-only directory in its place
|
||||
BLOCK_DIR="$DIR/buf_sks_blocked"
|
||||
mkdir -p "$BLOCK_DIR"
|
||||
chmod 500 "$BLOCK_DIR"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "sks_blocked" "PAYLOAD" 2>&1
|
||||
RC=$?
|
||||
echo "RC:$RC"
|
||||
|
||||
# Clean up permissions so test sandbox can clean up
|
||||
chmod 700 "$BLOCK_DIR"
|
||||
|
||||
# Check if any .tmp files were left behind
|
||||
ls -a "$DIR" | grep -q '\.tmp$' && echo "TEMP:LEAKED" || echo "TEMP:CLEAN"
|
||||
""")
|
||||
assert "RC:1" in res.stdout
|
||||
assert "TEMP:CLEAN" in res.stdout
|
||||
|
||||
|
||||
# X-12 — Scoped GC preserves unrelated dot-files
|
||||
def test_a3_unrelated_dot_files_preserved(mam_sandbox):
|
||||
res = _bash(mam_sandbox, """
|
||||
DIR="$WORKSPACE_ROOT/.mam/shim"
|
||||
mkdir -p "$DIR"
|
||||
DOT_OTHER="$DIR/.some_other_dot_file"
|
||||
: > "$DOT_OTHER"
|
||||
touch -t 202501010000 "$DOT_OTHER"
|
||||
|
||||
SHIM="$DIR/herdr"
|
||||
"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA"
|
||||
|
||||
if [ -f "$DOT_OTHER" ]; then
|
||||
echo "DOT_OTHER:PRESERVED"
|
||||
else
|
||||
echo "DOT_OTHER:DELETED"
|
||||
fi
|
||||
""")
|
||||
assert "DOT_OTHER:PRESERVED" in res.stdout
|
||||
Reference in New Issue
Block a user