Add shared managed lock broker

2026-10-07 02:23:39 +00:00
parent 18676070b2
commit 470c7fb09b
16 changed files with 1583 additions and 120 deletions
@@ -1,7 +1,7 @@
 ---
 name: gitea-code-review
 description: Spec pass된 Gitea PR의 코드 품질과 안전성을 검토한다.
-version: 0.1.1
+version: 0.1.2
 author: genichin, Hermes Agent
 license: MIT
 platforms: [linux]
@@ -15,6 +15,11 @@ metadata:

 `구현 → spec pass → code review`의 독립 2차 리뷰를 수행한다. 승인된 요구사항을 바꾸거나 직접 구현하지 않고, 코드 결함과 검증 부족을 근거로 판정한다.

+
+## Managed-lock broker gate
+
+Bootstrap 이후 code review는 canonical [Repository cache 운영 지침](../../../../operations/repository-cache.md)의 공통 managed-lock broker 하나를 사용한다. Checkout 밖 pre-created mode `0700` role-owned runtime directory와 unique handle/socket으로 tracked `serve`를 시작하고 ready JSON을 기다린다. Client는 raw token을 inspect/log하지 않으며 public `request` API가 token을 캡슐화한다. Nested repository lease는 같은 handle의 `acquire_repository`/`release_repository` request로만 관리하고 TTL 전에 `renew`한다. 정상·오류·cancel에서 repository release(보유 시) → Issue `release` → broker wait/reap → `probe` `free=true` 확인을 검증하기 전에는 완료·로그·보고하지 않는다. Raw `flock ... sleep`, lock-file unlink와 client-side lock holder는 금지한다. Native publication은 이 broker lease를 사용하며 adapter의 compatibility bridge는 canonical protocol을 우회하지 않는다.
+
 ## 사용 시점과 정본

 - Exact `owner/repository`, source Issue와 PR을 지정해 코드 리뷰 또는 재리뷰를 요청받았을 때 사용한다.
@@ -47,11 +52,11 @@ metadata:

 별도 워크트리는 만들지 않는다. 기존 impl Issue 워크트리는 다음 조건을 모두 만족할 때 **검증 전용으로 대여**한다. 구현 재개가 아니므로 `state.json`의 `created_by_role`은 impl role로 확인·보존하고 review role로 바꾸지 않는다.

-1. Workflow 동안 exact Issue lock을 유지한다. Cache·registration 검증에는 `issue lock → repository lock` 순서를 사용하고, 테스트 전에 repository lock만 해제한다.
+1. Workflow 동안 broker가 관리하는 exact Issue lease를 유지한다. Cache·registration 검증에는 같은 handle의 `acquire_repository` request를 사용하고, 테스트 전에 `release_repository` request를 보낸다.
 2. Canonical path·권한·state identity·common Git directory·detached/locked registration을 검증한다. HEAD는 현재 PR head와 같고 미완료 Git operation과 tracked 변경이 없어야 한다.
 3. Untracked 파일, dependency 환경과 설정이 검증을 오염시키지 않는지 확인한다. 설명할 수 없는 상태는 reset·clean·stash·checkout으로 복구하지 않고 중단한다.
 4. Source·test·Git refs·state는 수정하지 않는다. 테스트 출력·fixture·DB는 격리된 임시 경로를 사용하고 운영 데이터·credential을 사용하지 않는다. 파일 수정이나 위험한 외부 쓰기가 필요한 명령은 실행하지 않는다.
-5. 실행 전후 HEAD와 tracked/untracked 상태를 비교한다. 현재 실행이 만든 자원만 정리하고, 모든 종료 경로에서 lock을 역순 해제한 뒤 non-blocking 재획득·해제로 해제를 확인한다.
+5. 실행 전후 HEAD와 tracked/untracked 상태를 비교한다. 현재 실행이 만든 자원만 정리하고, 모든 종료 경로에서 canonical broker cleanup(보유한 `release_repository` → `release` → tracked broker wait/reap → `probe` `free=true`)을 검증한다.

 안전한 대여가 불가능하면 CI·기존 근거를 사용하고 직접 실행 불가 사유를 보고한다. 필수 검증 근거가 부족하면 승인하지 않는다. 독립성은 reviewer의 판단과 직접 확인에 있으며 별도 checkout 자체가 필수는 아니다.

@@ -8,7 +8,7 @@ This adapter is a publication guard, not a substitute for the main reviewer's co

 - Python API: `build_payload(plan)`, `validate_snapshot(plan, snapshot)`, `verify_publication(plan, before, after, exact)`, and `publish(plan, baseline, api, lease, journal_directory)`.
 - REST bridge: `ApiAdapter(authenticated_request, origin=..., credential_source=...)`.
- Existing lease bridge: `HeldIssueLease(issue_fd, absolute_cache_policy, deadline_monotonic)`.
+- Primary lease bridge: `BrokerIssueLease(broker_handle, absolute_cache_policy, deadline_monotonic)` backed by `managed_lock_broker.py`; `HeldIssueLease(issue_fd, absolute_cache_policy, deadline_monotonic)` remains compatibility-only for already-admitted legacy callers. Broker requests are limited to `status`, `renew`, `acquire_repository`, `release_repository` and `release`; renew before expiry and use canonical release/wait/reap/probe cleanup.
 - Read-only CLI: `preflight --plan PLAN --baseline SNAPSHOT` and `verify --plan PLAN --before BEFORE --after AFTER --exact EXACT`.
 - There is intentionally **no live `publish` CLI**. Live publication uses the Python API with the workflow's existing authenticated transport and still-held Issue lock. Never invent a `capture`, `build`, `publish`, or `submit` command or guess a legacy helper schema.

@@ -39,10 +39,10 @@ Keep `gitea-access.md` and `repository-cache.md` authoritative. Before calling `
 1. Admit the canonical, existing role-owned repository cache and impl-created, registered detached/locked worktree. Complete the independent code review and collect evidence on the exact source head.
 2. Authenticate **one** transport from the current role's `$HERMES_HOME/.env` `GITEA_TOKEN`, or an explicitly authorized role-scoped external secret source. No ambient-token fallback, cross-profile credentials, credential-bearing Git URLs or anonymous downgrade is allowed.
 3. Validate the exact fixed origin `https://gitea.jsfamily.quest/api/v1` and `GET /user` account ID/login. The transport must verify TLS, reject redirects, bound network timeouts and **not retry mutation requests**. The bridge's provenance labels are assertions supplied by this already-validated caller, not a mechanism to load or certify an arbitrary client.
-4. Keep the Issue `fcntl.flock` exclusive descriptor owned by this process open throughout publication/readback. Pass a live monotonic deadline, not an old PID/`ready.json` flag. `HeldIssueLease` checks exact canonical Issue lock inode, owner/mode, worktree state/registration, clean HEAD and deadline. It never releases or closes the caller's descriptor. The caller must do so in its normal `finally` cleanup. A custom supervisor lease must implement the same invariants: `check(plan)` returns `None` only on success and otherwise raises; boolean/dict success flags are unsupported.
+4. Use the common broker Issue lease throughout publication/readback. Pass a live monotonic deadline, not an old PID/`ready.json` flag. `BrokerIssueLease` checks broker status and canonical worktree invariants; renew before expiry. `HeldIssueLease` is compatibility-only and may be used only by an already-admitted legacy caller, never to create a second lock protocol. The caller must run canonical release/wait/reap/probe cleanup. A custom supervisor lease must implement the same invariants: `check(plan)` returns `None` only on success and otherwise raises; boolean/dict success flags are unsupported.
 5. Put the journal outside every source checkout in a trusted artifact parent. Use the **same journal for every retry/resume of this plan**, not a new directory after a timeout. The adapter creates mode 0700 directories and 0600 atomic/fsynced artifacts, rejects symlinks/hardlinks/foreign or unsafe modes, and holds a separate nonblocking journal mutex. Redact secrets before constructing plans/artifacts; do not log request headers, raw transport exceptions or token values. Journals contain complete source snapshots and must remain private.

-The current helper is strict about worktree child directories: group/world-writable checkout paths are rejected. Do not change a real worktree's permissions to force publication; stop and reconcile an admission/publisher compatibility conflict. The built-in lease bridge is Linux-only: it requires an existing whole-file exclusive `FLOCK` owned by this process in `/proc/self/fdinfo`, and never acquires or reacquires that lock. It uses a root-owned, non-group/world-writable `/usr/bin/git` with a fixed minimal environment, not the caller's `PATH`.
+The current helper is strict about worktree child directories: group/world-writable checkout paths are rejected. Do not change a real worktree's permissions to force publication; stop and reconcile an admission/publisher compatibility conflict. The legacy compatibility bridge is Linux-only: it validates an already-admitted broker-held whole-file lease represented by an exclusive `FLOCK` owned by this process in `/proc/self/fdinfo`, and never acquires or reacquires that resource. New callers use `BrokerIssueLease` and the broker public request API. It uses a root-owned, non-group/world-writable `/usr/bin/git` with a fixed minimal environment, not the caller's `PATH`.

 These guards assume a cooperative, trusted role-local filesystem and transport. They do not isolate a malicious process with the same UID or pin every filesystem ancestor across subprocess path resolution. At-most-once creation is scoped to the retained journal and held canonical Issue lease, **not** arbitrary alternate journal paths, unrelated publishers, or multiple hosts. Do not share one lease across simultaneous publication callers or change the journal after an uncertain attempt.

@@ -60,7 +60,7 @@ api = native_review.ApiAdapter(
    origin="https://gitea.jsfamily.quest/api/v1",
    credential_source="$HERMES_HOME/.env:GITEA_TOKEN",
 )
-lease = native_review.HeldIssueLease(issue_fd, absolute_cache_policy, deadline_monotonic)
+lease = native_review.BrokerIssueLease(broker_handle, absolute_cache_policy, deadline_monotonic)
 baseline = api.snapshot(plan["target"])
 native_review.validate_snapshot(plan, baseline)
 result = native_review.publish(plan, baseline, api, lease, journal_directory)
@@ -14,6 +14,14 @@ _SPEC_PATH = (Path(__file__).resolve().parents[4] /
 _spec = importlib.util.spec_from_file_location('gitea_native_spec_support', _SPEC_PATH)
 _support = importlib.util.module_from_spec(_spec)
 _spec.loader.exec_module(_support)
+_LOCK_PATH = Path(__file__).resolve().parents[5] / 'lib/managed_lock.py'
+_lock_spec = importlib.util.spec_from_file_location('gitea_managed_lock', _LOCK_PATH)
+_locks = importlib.util.module_from_spec(_lock_spec)
+_lock_spec.loader.exec_module(_locks)
+_BROKER_PATH = Path(__file__).resolve().parents[5] / 'lib/managed_lock_broker.py'
+_broker_spec = importlib.util.spec_from_file_location('gitea_managed_lock_broker', _BROKER_PATH)
+_broker = importlib.util.module_from_spec(_broker_spec)
+_broker_spec.loader.exec_module(_broker)
 ValidationError = _support.ValidationError
 loads_json = _support.loads_json
 collect_pages = _support.collect_pages
@@ -646,83 +654,158 @@ class ApiAdapter:
        return data


+def _validate_registered_source(plan, policy_path, deadline_monotonic):
+    import math, os, stat, subprocess, time
+    if type(deadline_monotonic) not in [int, float] or not math.isfinite(deadline_monotonic):
+        fail('invalid or expired Issue lease')
+    t = plan['target']
+    info = _support.validate_cache_paths(policy_path, t['owner'], t['repository'], t['issue'])
+    wt = Path(info['repository']) / 'worktrees' / f'issue-{t["issue"]}' / 'checkout'
+    opened = []
+    def close_opened():
+        while opened:
+            try: os.close(opened.pop())
+            except OSError: pass
+    def reject(message):
+        close_opened()
+        fail(message)
+    def open_dir(path):
+        parts = path.parts
+        fd = os.open(parts[0], os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC)
+        opened.append(fd)
+        try:
+            for part in parts[1:]:
+                nxt = os.open(part, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC,
+                              dir_fd=fd)
+                opened.append(nxt); os.close(fd); opened.remove(fd); fd = nxt
+            return fd
+        except BaseException:
+            close_opened(); raise
+    try:
+        worktrees_fd = open_dir(wt.parent.parent)
+        issue_fd = os.open(wt.parent.name, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC,
+                           dir_fd=worktrees_fd)
+        opened.append(issue_fd)
+        checkout_fd = os.open(wt.name, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC,
+                              dir_fd=issue_fd)
+        opened.append(checkout_fd)
+        for directory_fd in [worktrees_fd, issue_fd, checkout_fd]:
+            st = os.fstat(directory_fd)
+            if (not stat.S_ISDIR(st.st_mode) or st.st_uid != os.getuid()
+                    or st.st_mode & (stat.S_IWGRP | stat.S_IWOTH)):
+                reject('unsafe existing worktree path')
+        state_fd = os.open('state.json', os.O_RDONLY | os.O_NOFOLLOW | os.O_CLOEXEC, dir_fd=issue_fd)
+        opened.append(state_fd)
+        git_pointer_fd = os.open('.git', os.O_RDONLY | os.O_NOFOLLOW | os.O_CLOEXEC, dir_fd=checkout_fd)
+        opened.append(git_pointer_fd)
+        state_st = os.fstat(state_fd); git_st = os.fstat(git_pointer_fd)
+        if not stat.S_ISREG(state_st.st_mode) or state_st.st_uid != os.getuid() or stat.S_IMODE(state_st.st_mode) != 0o600:
+            reject('unsafe worktree state')
+        if not stat.S_ISREG(git_st.st_mode) or git_st.st_uid != os.getuid():
+            reject('invalid worktree Git pointer')
+        chunks = []
+        while chunk := os.read(state_fd, 65536):
+            chunks.append(chunk)
+        try:
+            state = loads_json(b''.join(chunks).decode('utf-8'))
+        except Exception:
+            close_opened()
+            raise
+        os.lseek(git_pointer_fd, 0, os.SEEK_SET)
+        pointer = os.read(git_pointer_fd, 4096).decode('utf-8').strip()
+        if not pointer.startswith('gitdir: '):
+            reject('invalid worktree Git pointer')
+        worktree_git_fd = open_dir(Path(pointer[8:]))
+    except OSError:
+        close_opened()
+        raise ValidationError('unsafe existing worktree path') from None
+
+    if (type(state) is not dict or state.get('schema') != 'gitea-agent.worktree/v1' or state.get('owner') != t['owner']
+            or state.get('repository') != t['repository'] or type(state.get('issue')) is not int
+            or state['issue'] != t['issue'] or state.get('created_by_role') != 'gitea-impl-agent'
+            or state.get('mode') != 'detached'):
+        reject('worktree registration identity mismatch')
+
+    git_binary = '/usr/bin/git'; binary_stat = Path(git_binary).lstat()
+    if not stat.S_ISREG(binary_stat.st_mode) or binary_stat.st_uid != 0 or binary_stat.st_mode & 0o022:
+        reject('untrusted system Git binary')
+    env = {'PATH': '/usr/bin:/bin', 'GIT_CONFIG_GLOBAL': '/dev/null', 'GIT_CONFIG_NOSYSTEM': '1', 'GIT_OPTIONAL_LOCKS': '0'}
+    git_dir_fd = open_dir(Path(info['repository_git']))
+    def git(*args):
+        try:
+            process = subprocess.run([git_binary, '-c', 'core.fsmonitor=false', '-c', 'core.hooksPath=/dev/null',
+                                      '-c', 'safe.directory=/proc/self/fd/' + str(checkout_fd),
+                                      '--git-dir=/proc/self/fd/' + str(worktree_git_fd),
+                                      '--work-tree=/proc/self/fd/' + str(checkout_fd), *args], capture_output=True, text=True, env=env, timeout=15,
+                                      pass_fds=(checkout_fd, worktree_git_fd, git_dir_fd))
+        except Exception:
+            close_opened()
+            raise ValidationError('read-only Git check unavailable') from None
+        if process.returncode or len(process.stdout) > 1000000:
+            reject('read-only Git check failed')
+        return process.stdout
+    if git('rev-parse', 'HEAD').strip() != t['head']:
+        reject('worktree HEAD mismatch')
+    if git('status', '--porcelain=v1', '--untracked-files=all').strip():
+        reject('existing worktree is not clean')
+    if git('rev-parse', '--path-format=absolute', '--git-common-dir').strip() != info['repository_git']:
+        reject('worktree common Git directory mismatch')
+    records = git('--git-dir=/proc/self/fd/' + str(git_dir_fd), 'worktree', 'list', '--porcelain', '-z').split('\0\0')
+    matches = [r.split('\0') for r in records if r.startswith('worktree ' + str(wt) + '\0')]
+    if len(matches) != 1 or 'detached' not in matches[0] or not any(x == 'locked' or x.startswith('locked ') for x in matches[0]):
+        reject('worktree detached/locked registration mismatch')
+    close_opened()
+    if time.monotonic() >= deadline_monotonic:
+        fail('Issue lease expired during source checks')
+
+
 class HeldIssueLease:
    """Validate a caller-owned Issue lock FD; never close/release the caller's FD."""
    def __init__(self, issue_fd, policy_path, deadline_monotonic):
-        self.issue_fd = issue_fd
-        self.policy_path = policy_path
-        self.deadline_monotonic = deadline_monotonic
+        self.issue_fd = issue_fd; self.policy_path = policy_path; self.deadline_monotonic = deadline_monotonic

    def check(self, plan):
-        import math, os, stat, subprocess, time
+        import math, time
        validate_plan(plan)
        if (type(self.issue_fd) is not int or type(self.deadline_monotonic) not in [int, float]
                or not math.isfinite(self.deadline_monotonic) or time.monotonic() >= self.deadline_monotonic):
            fail('invalid or expired Issue lease')
-        t = plan['target']
-        info = _support.validate_cache_paths(self.policy_path, t['owner'], t['repository'], t['issue'])
+        t = plan['target']; info = _support.validate_cache_paths(self.policy_path, t['owner'], t['repository'], t['issue'])
        lock = Path(info['locks']) / f'issue-{t["issue"]}.lock'
        try:
-            current = os.fstat(self.issue_fd); disk = lock.lstat()
-            if (not stat.S_ISREG(current.st_mode) or not stat.S_ISREG(disk.st_mode)
-                    or current.st_uid != os.getuid() or stat.S_IMODE(current.st_mode) != 0o600
-                    or current.st_nlink != 1 or (current.st_dev, current.st_ino) != (disk.st_dev, disk.st_ino)):
-                fail('invalid Issue lock descriptor')
-            # Linux fdinfo reports locks on this open file description. Do not
-            # acquire/reacquire here: an opened but unheld FD is not a lease.
-            records = [line.split() for line in Path(f'/proc/self/fdinfo/{self.issue_fd}').read_text().splitlines()]
-            device_inode = f'{os.major(current.st_dev):02x}:{os.minor(current.st_dev):02x}:{current.st_ino}'
-            if not any(len(row) == 9 and row[0] == 'lock:'
-                       and row[2:6] == ['FLOCK', 'ADVISORY', 'WRITE', str(os.getpid())]
-                       and row[6:] == [device_inode, '0', 'EOF'] for row in records):
+            if not _locks.validate_held_flock(self.issue_fd, lock):
                fail('Issue lock is not already held by this process')
-        except OSError:
+        except (OSError, _locks.LockValidationError):
            raise ValidationError('Issue lock unavailable') from None
-        wt = Path(info['repository']) / 'worktrees' / f'issue-{t["issue"]}' / 'checkout'
-        for directory in [wt.parent.parent, wt.parent, wt]:
-            st = directory.lstat()
-            if not stat.S_ISDIR(st.st_mode) or st.st_uid != os.getuid() or st.st_mode & (stat.S_IWGRP | stat.S_IWOTH):
-                fail('unsafe existing worktree path')
-        state_file = wt.parent / 'state.json'; st = state_file.lstat()
-        if not stat.S_ISREG(st.st_mode) or st.st_uid != os.getuid() or stat.S_IMODE(st.st_mode) != 0o600:
-            fail('unsafe worktree state')
-        state = loads_json(state_file.read_text(encoding='utf-8'))
-        if (type(state) is not dict or state.get('schema') != 'gitea-agent.worktree/v1' or state.get('owner') != t['owner']
-                or state.get('repository') != t['repository'] or type(state.get('issue')) is not int
-                or state['issue'] != t['issue'] or state.get('created_by_role') != 'gitea-impl-agent'
-                or state.get('mode') != 'detached'):
-            fail('worktree registration identity mismatch')
-        st = (wt / '.git').lstat()
-        if not stat.S_ISREG(st.st_mode) or st.st_uid != os.getuid():
-            fail('invalid worktree Git pointer')
-        git_binary = '/usr/bin/git'
-        binary_stat = Path(git_binary).lstat()
-        if not stat.S_ISREG(binary_stat.st_mode) or binary_stat.st_uid != 0 or binary_stat.st_mode & 0o022:
-            fail('untrusted system Git binary')
-        env = {'PATH': '/usr/bin:/bin', 'GIT_CONFIG_GLOBAL': '/dev/null',
-               'GIT_CONFIG_NOSYSTEM': '1', 'GIT_OPTIONAL_LOCKS': '0'}
-        def git(*args):
-            try:
-                process = subprocess.run([git_binary, '-c', 'core.fsmonitor=false', '-c', 'core.hooksPath=/dev/null',
-                                          '-c', 'safe.directory=' + str(wt), *args],
-                                         capture_output=True, text=True, env=env, timeout=15)
-            except Exception:
-                raise ValidationError('read-only Git check unavailable') from None
-            if process.returncode or len(process.stdout) > 1000000:
-                fail('read-only Git check failed')
-            return process.stdout
-        if git('-C', str(wt), 'rev-parse', 'HEAD').strip() != t['head']:
-            fail('worktree HEAD mismatch')
-        if git('-C', str(wt), 'status', '--porcelain=v1', '--untracked-files=all').strip():
-            fail('existing worktree is not clean')
-        if git('-C', str(wt), 'rev-parse', '--path-format=absolute', '--git-common-dir').strip() != info['repository_git']:
-            fail('worktree common Git directory mismatch')
-        records = git('--git-dir=' + info['repository_git'], 'worktree', 'list', '--porcelain', '-z').split('\0\0')
-        matches = [r.split('\0') for r in records if r.startswith('worktree ' + str(wt) + '\0')]
-        if len(matches) != 1 or 'detached' not in matches[0] or not any(x == 'locked' or x.startswith('locked ') for x in matches[0]):
-            fail('worktree detached/locked registration mismatch')
-        if time.monotonic() >= self.deadline_monotonic:
-            fail('Issue lease expired during source checks')
+        _validate_registered_source(plan, self.policy_path, self.deadline_monotonic)
+
+
+class BrokerIssueLease:
+    """Validate an authenticated shared managed-lock broker Issue lease."""
+    def __init__(self, handle_path, policy_path, deadline_monotonic):
+        self.handle_path = handle_path; self.policy_path = policy_path; self.deadline_monotonic = deadline_monotonic
+
+    def check(self, plan):
+        import math, time
+        validate_plan(plan)
+        if (type(self.deadline_monotonic) not in [int, float] or not math.isfinite(self.deadline_monotonic)
+                or time.monotonic() >= self.deadline_monotonic):
+            fail('invalid or expired Issue lease')
+        t = plan['target']
+        try:
+            handle = _broker.read_handle_metadata(self.handle_path)
+            status = _broker.request_handle(self.handle_path, 'status')
+        except Exception:
+            raise ValidationError('broker Issue lease unavailable') from None
+        if (type(status) is not dict or status.get('status') not in ('held', 'repository-held')
+                or status.get('owner') != t['owner'] or status.get('repository') != t['repository']
+                or status.get('issue') != t['issue'] or status.get('role') != 'gitea-review-agent'
+                or type(status.get('pid')) is not int or status.get('pid') != handle.get('pid')
+                or type(handle.get('pid')) is not int or status.get('deadline') != handle.get('deadline')
+                or type(status.get('deadline')) not in [int, float]
+                or status.get('deadline') > self.deadline_monotonic or time.monotonic() >= status.get('deadline')):
+            fail('broker Issue lease identity mismatch')
+        _validate_registered_source(plan, self.policy_path, self.deadline_monotonic)


 def main(argv=None):