Code Review

코드 리뷰 결과

이 문서는 pytmux 코드베이스 전체에 대해 수행한 엔지니어링 코드 리뷰/감사의 공개 요약이다. 규모(약 64k LOC, 151개 .py)의 코어(pytmuxlib/) + 플러그인(plugins/) + 테스트(tests/)를 정독해, 각 관점별 강점과 개선 가능성을 평가하고, 발견 항목을 차례대로 수정한 결과를 정리한다.

요약: 발견된 모든 High/Medium 등급 항목(전부 기능 버그)을 수정 완료했으며, 헤드리스 단위 테스트는 846개 → 874개 통과로 증가했다(회귀 테스트 동반 추가).

---

1. 검수 범위

코어·플러그인·테스트 전체를 다음 하위 시스템 단위로 정독했다.

• 전송/프로토콜: 와이어 프로토콜(protocol.py), IPC 보안 경계(ipc.py), VT 파서(vtparse.py).
• 서버 코어: 합성 진입(server.py), 명령/플러시/브로드캐스트(serverio.py), PTY 백엔드(pty_backend.py/conpty.py), 아웃오브프로세스 pty-host(ptyhost*.py), 트리/레이아웃(servertree.py), 세션유지 재시작(serverpersist.py), 페더레이션(serverremote.py).
• 데이터 계층: 토큰 사용량 SQLite 저장소 및 마이그레이션, 직렬화 경로(model.py).
• 클라이언트: App 본문/믹스인(client.py), 위젯/상태바(clientwidgets.py), 렌더·유틸.
• 플러그인: 레지스트리 훅 계약(plugins/__init__.py), Claude 연동·오버레이류 플러그인.

선행 보안 검토 및 코드 감사에서 이미 적용된 완화책은 재보고하지 않고, 적용 상태를 실제 코드와 대조 검증한 뒤 net-new 발견에 집중했다.

---

2. 검수 방법론

• 다관점 정독: 5개 관점(유지보수성·안정성·보안·LLM 친화성·동시작업 대응)별로 독립적으로 코드를 직접 읽었다.
• 정량 측정 병행: grep/wc/AST 기반 측정으로 docstring·타입 어노테이션 커버리지, except 패턴 분포, God 클래스 메서드 수 등을 수치화했다.
• 데이터 흐름 추적: 비신뢰 입력(패널 출력·클라 명령)이 어디까지 도달하는지 추적해 신뢰 경계를 검증했다.
• 선행 결론 대조: 과거 검토 결과와 대조해 오탐·재보고를 회피하고, "적용됨"으로 기록된 완화책이 실제 코드에 존재하는지 확인했다.
• 게이트: 동적 익스플로잇·벤치마크는 미수행. 개선 적용 시 전체 테스트 스위트 통과를 필수 게이트로 삼고, 보안 건은 회귀 테스트 추가를 의무화했다.
• 제약: 실 PTY·실 ConPTY(Windows)·실 외부 CLI 패널은 헤드리스 드라이버로 검증 불가 영역으로, 정적 분석으로 다룬다.

---

3. 발견 분류 및 심각도

발견은 5개 관점으로 분류하고, 각 관점에 등급을 매겼다.

관점	등급	한 줄 요약
유지보수성	B−	플러그인 경계·코어 위생 우수 vs 거대 파일/God 클래스·테스트 인프라 취약·문서 부채
안정성	B	핵심 IPC/프로토콜/파서/DB 견고 vs PTY-host·페더레이션 경계 하드닝 필요
보안	A−	성숙도 매우 높음. 단 자동 승격 경로에 net-new High 1건
LLM 친화성	B+	명시 디스패치·집중 훅 계약·풍부한 docstring 우수 vs 거대 파일·스크래핑 결합
동시작업 대응	B	단일루프 + 동기 구조변경으로 모델 동시성 견고 vs 송신 fan-out 계층 최약

심각도 버킷은 High / Medium / Low(보안은 Critical/High/Med/Low)를 사용한다.

총평

약 64k LOC 규모 대비 아키텍처 규율이 높다. 공통적으로 확인된 강점:

• 플러그인 경계의 청결함: "디렉토리 삭제 = 조용한 비활성화" 불변식이 코어 전역에서 지켜진다. 코어→플러그인은 레지스트리 훅 + getattr 가드로만 닿고, 코어가 플러그인을 직접 import하지 않는다. test_plugin_contract.py가 이를 강제한다.
• 코어 위생: 코어 전수 grep에서 TODO/FIXME/XXX/HACK 및 주석처리 코드블록 0건.
• 견고한 와이어 프로토콜·VT 파서·IPC 보안 경계와 상시 퍼징.
• 수백 개의 헤드리스 단위 테스트.

남은 부채는 두 갈래로 수렴한다 — (가) 거대 파일/God 클래스(유지보수·LLM·동시편집 충돌을 동시 악화)와 (나) 아웃오브프로세스 경계(PTY-host·페더레이션)의 실패경로·타임아웃·송신 fan-out 견고성.

---

4. 대표 발견과 수정

각 발견은 회귀 테스트 추가 + 전체 스위트 통과를 동반해 수정 완료했다.

4.1 보안 (Security)

[High] 위조 패널 출력으로 임의 호스트 아웃바운드 연결 + 키입력 가로채기

• 위치: 중첩 attach(NEST) 자동 승격 경로 — serverpty.py(패널 raw 출력 DCS 스캔), serverremote.py(엔드포인트 직결 분기), 기본값 자동 승격 ON.
• 위협모델: 패널에서 신뢰 불가 프로그램 출력을 보는 것만으로 성립한다(무인가 IPC 접근 불요).
• 시나리오: ① DCS 스캐너가 모든 패널 raw 출력에 무조건 실행된다(provenance 게이트 없음). ② 악성 출력(조작 파일 표시, 손상된 원격 셸)이 특수 DCS 시퀀스를 emit해 패널의 원격 목적지 필드를 임의값(tcp:evil.example:9999)으로 설정한다. ③ 자기보고 값도 동일하게 맞춰 호스트 일치 가드를 자명하게 통과시킨다. ④ 엔드포인트 직결 분기가 ssh 옵션 인젝션 방어 가드를 우회해 실제 아웃바운드 TCP 연결을 연다. ⑤ 클라이언트가 그 탭을 보면 라이브 키입력이 공격자 서버로 릴레이된다(MITM/피싱).
• 핵심 결함: "attach 인자는 래퍼가 기록한 목적지 필드만 쓰므로 안전"이라는 전제가 거짓이었다. 그 목적지 필드 자체가 신뢰 불가 패널 출력(DCS)에서 설정되므로, 로컬 엔드포인트 접두는 ssh 옵션 인젝션 가드를 엔드포인트 분기로 완전히 우회한다.
• 수정: (핵심) 패널 출력 발 목적지는 로컬 엔드포인트 직결을 금지하고 항상 ssh 호스트로만 해석해 가드 경유를 강제. provenance 토큰을 도입해 비로컬 엔드포인트 직결을 차단했다. (심층방어) 자동 승격에도 허용목록을 적용하는 옵션 제공.

선행 보안 검토에서 적용된 완화책들(토큰 인증, peer-UID 검증, 상태 디렉토리 선점 거부, 0600/0700 권한, ssh 옵션 인젝션 거부, SQL 전수 파라미터화, 역직렬화 부재, subprocess 전수 argv, DCS 정규식 길이 상한에 의한 ReDoS 차단)은 모두 실제 코드에 존재함을 확인했다.

4.2 안정성 (Stability)

• [H1] PTY write 부분 write/EAGAIN 미처리 (pty_backend.py): 논블로킹 fd인데 단일 os.write 호출 + 반환값 무시. 큰 페이스트/포화 시 입력 무음 유실 또는 BlockingIOError 무음 드롭. 수정: memoryview 잔여 루프 write + EINTR 재시도 + EAGAIN 시 양보/버퍼링.
• [H2] PTY-host read 루프 콜백 예외 광역 흡수 (ptyhostclient.py): 한 패널 콜백 예외가 루프를 끊어 host는 멀쩡한데 전 패널 연결이 죽은 것으로 오인되고 로깅도 0건. 수정: 콜백을 per-frame try/except로 격리하고, 진짜 단절일 때만 break.
• [H3] ConPTY spawn 실패 시 핸들/프로세스 누수 (conpty.py): 생성자가 핸들 생성 후 spawn 예외 시 close() 없이 버려지고, 상위 안전망이 로깅 0으로 삼켜 "조용한 폴백 + 매번 누수". 수정: spawn을 try로 감싸 실패 시 close() + 폴백 직전 디버그 로깅.
• [H4] 원격 전송 ssh 서브프로세스 reap/cleanup 누락 (serverremote.py): 타임아웃/핸드셰이크 실패 경로에서 ssh 자식 프로세스와 fd가 정리되지 않아 좀비/fd 누수. 무개행 초과 입력에 대한 LimitOverrunError도 미포착. 수정: try/finally로 모든 실패경로에서 프로세스 kill + wait 보장, except에 LimitOverrunError/ValueError 추가.
• [H5] 자동 재연결이 "hello 성공"을 "병합 성공"으로 오판 (serverremote.py): 대화형 attach는 실제 첫 상태 도착을 확인하나, 재연결 루프는 hello 송신 직후 True를 보고 "재연결됨"을 알린다. 업스트림 pty-host가 wedge되면 매번 잘못된 성공 통지 + 화면 미수신 + 리소스 누적. 수정: 재연결 성공 기준을 첫 상태 도착 확인으로 강화(대화형 경로와 정합).
• Medium 다수: 이벤트 루프 블로킹 reap을 executor로 오프로드, PTY-host/페더레이션 read 타임아웃·keepalive 도입(half-open wedge 영구 hang의 구조적 근원 봉합), 스레드 join, 복원 부분실패 시 fd 정리, 재시작 상태파일 원자적 쓰기(temp + os.replace) 등을 수정.

4.3 동시작업 대응 (Concurrency)

서버는 단일 스레드 asyncio 루프이며 구조 변경 연산이 모두 동기라 모델 동시변경은 락 없이 안전하다. 위험은 await로 갈라지는 read-then-write와 공유 writer의 비직렬화 fan-out에 집중된다.

• [H-1] Fan-out 송신 per-writer 직렬화 부재 → 프레임 순서역전 (serverio.py 등): 같은 writer에 flush 루프와 다중-await 전체전송이 동시에 미결되어 프레임이 혼입될 수 있다(다음 flush로 자가교정되나 일시 깨짐). 수정: 클라이언트 연결별 송신 직렬화(락/큐) 도입.
• [H-2] 느린 소비자가 flush 루프 전체를 head-of-line 블록 (serverio.py): 한 클라의 TCP 수신 윈도 포화 시 drain()이 무기한 대기해 모든 클라·모든 세션 렌더가 정지. 수정: 클라별 송신 분리 또는 타임아웃 + high-water 초과 시 느린 소비자 마킹/드롭.
• [M-1/M-2] 원격 입력 릴레이 직렬화, autorename 루프의 await 사이 stale 모델 write 가드.

4.4 유지보수성 (Maintainability)

• [H] 코어 옵션 기본값↔영속 키 손중복으로 인한 실버그: 같은 옵션 집합이 로드 경로와 저장 경로에 수기로 두 번 나열되어 있었고, 한 옵션(usage_refresh_sec)이 저장 경로에 누락되어 사용자가 바꿔도 다음 기동에 리셋되었다. 수정: 플러그인 측에서 검증된 단일 테이블 패턴 ((key, default, cast) 튜플)을 코어에 적용해 드리프트를 구조적으로 차단 + load↔save 대칭 테스트 추가.
• [H] 번역 우회 하드코딩 한국어 리터럴: 위젯 리터럴이 i18n을 우회해 영어 사용자에게 한글이 노출되었다(탭바 라벨 등). 카탈로그만 검사하던 기존 테스트의 사각지대였다. 수정: 노출 리터럴 번역 처리 + 소스 리터럴 lint 테스트 추가.
• 죽은/이중 정의 정리: 호출 0건 메서드 삭제, 중복 헬퍼 통합.
• stale 주석/숫자 정정: 코드와 모순되는 주석(파서 기본값 표기 등)과 산문 속 stale 수치를 불변식 위임으로 정정.

4.5 LLM 친화성 (LLM-Friendliness)

강점: 동적 디스패치가 거의 없어 명령 라우팅을 문자열 grep으로 직행 가능, 집중된 훅 계약, 빠른 헤드리스 회귀 검증.

• [발견] 외부 CLI 영어 TUI 스크래핑 결합: 핵심 기능(상태/토큰/계정)이 외부 CLI의 영어 TUI를 정규식/문자열로 긁어 문구 변경에 취약하다. 단 방어가 상당하다(이메일 신호만 신뢰 + 폴백 + ReDoS 상한 + 실캡처 회귀망). 수정/개선: 모델 화이트리스트를 외부화하고, 골든 픽스처 회귀를 보강.
• [개선] 거대 파일 + 동적 합성 메서드: 일부 서버 메서드가 런타임에 플러그인 믹스인으로 합성되어 정의 위치를 grep으로 못 찾는 문제에 호출 지점 포인터 주석을 추가. 루트 온보딩 문서(CLAUDE.md)를 신설해 빌드/테스트 명령, 훅 계약 위치, delete-to-disable 규약, 거대 파일/문서 Read 주의를 정리.

---

5. 해결 상태

• 수정 완료: 발견된 모든 High·Medium 등급 항목(전부 기능 버그)을 차례대로 수정했다. 각 수정은 회귀 테스트 추가 + 전체 테스트 스위트 통과를 게이트로 통과했다.
• 테스트 결과: 헤드리스 단위 테스트 846 → 874 passed, 0 failed.
• 보류 항목(사유 명시): 순수 cleanup이고 기능 이득이 0이며 실행 중 발견한 리스크가 큰 대규모 리팩터는 보류했다.
  • 거대 파일 분할/God 클래스 모듈 승격(client.py 등): 대상 파일이 병렬 작업으로 활발히 편집 중이라, 거대 분할이 진행 중 작업과 충돌할 위험이 커 전용 작업 시점으로 미뤘다.
  • 명령 디스패치 테이블화: 80여 분기가 단순 리프 디스패치가 아니라 공유 지역상태와 early-return 제어흐름이 얽힌 복잡 inline 로직이라, 드라이버 검증 불가 명령 경로의 회귀 위험 대비 이득이 스타일뿐이어서 보류.
  • 레거시 파서 경로 폐기: 추정보다 참조가 많고(다수 파일·hot path 포함), 파서 동등성 오라클 (native ≡ 레거시 상시 회귀)을 제거하게 되어, 드라이버 검증 불가 렌더러에서 오라클을 버리는 트레이드오프가 불리해 보류.
  • 타입힌트 전수화: cosmetic 등급으로, 공유 hot 파일의 churn/충돌만 키우고 이득이 미미해 보류.

권고: 보류된 대규모 리팩터는 대상 파일이 병렬 작업에서 풀린 뒤 전용 작업 시점에서, 레거시 파서 폐기는 파서 오라클 대체 검증 전략과 함께 진행한다.

---

관련 문서

• Security-Audit
• Project-Overview