Skip to content

🟢 N-5: escalation-ledger.py Windows fallback drops lock + O_NOFOLLOW #77

Description

@ComBba

Priority: 🟢 Nice-to-have (Windows-only, LOW)
Audit source: T2 Backend Expert (backend-architect)
Related file: plugins/preview-forge/hooks/escalation-ledger.py:107-114

Problem

The Windows branch of escalation-ledger.py's _lockfile() context manager:

  1. Yields the lock as a no-op (concurrent-write race window exposed)
  2. Does not apply O_NOFOLLOW (symlink redirect possible)

Evidence

# plugins/preview-forge/hooks/escalation-ledger.py:107-114
@contextmanager
def _lockfile(lock_path):
    if not _HAVE_FCNTL:
        # Windows fallback: no lock, no O_NOFOLLOW
        yield
        return
    # POSIX path: O_NOFOLLOW + flock
    lock_fd = os.open(...)
    ...

The docstring justifies it as "rarely concurrent" — acknowledging the POSIX concurrency case while staying silent about Windows.

Attack scenario (Windows-only)

  1. In a shared-home environment, an attacker pre-creates ~/.preview-forge/escalation-history.lock as a reparse point/junction
  2. The ledger uses os.replace to atomically replace history.json
  3. The reparse point redirects to the attacker's file → ledger contents land in the attacker's file (or the attacker's file becomes the ledger location via redirect)

Implications

  • POSIX (macOS/Linux): no impact (fcntl + O_NOFOLLOW)
  • Windows: TOCTOU + symlink defense both absent
  • Frequency: shared-user scenarios — uncommon, but unsafe by design

Because there is no T-12 Windows CI (Issue I-4), this behavior has 0 runtime verification.

Acceptance Criteria

Option A: a Windows-specific lock mechanism

  • Use msvcrt.locking() (Python stdlib, provides file locking on Windows):
    if not _HAVE_FCNTL:
        try:
            import msvcrt
        except ImportError:
            yield  # documented limitation
            return
        with open(lock_path, 'w') as f:
            msvcrt.locking(f.fileno(), msvcrt.LK_LOCK, 1)
            try:
                yield
            finally:
                msvcrt.locking(f.fileno(), msvcrt.LK_UNLCK, 1)

Option B: reparse-point validation

  • Call GetFileAttributesW and inspect the FILE_ATTRIBUTE_REPARSE_POINT bit — abort if set
  • If pywin32 would be required, that conflicts with LESSON 0.4 (zero-dep) — call directly via ctypes:
    import ctypes
    attrs = ctypes.windll.kernel32.GetFileAttributesW(str(lock_path))
    if attrs & 0x400:  # FILE_ATTRIBUTE_REPARSE_POINT
        raise OSError("refusing to lock reparse point")

Option C: explicit limitation documentation (minimum)

  • On entering the Windows fallback, emit a stderr warning:
    escalation-ledger: Windows fallback active — no concurrency lock.
    Multi-user shared home is NOT supported.
    
  • State "Windows multi-user environment is unsupported" in a LESSON entry or the README

Cross-references

Notes

No impact for POSIX users. Worth preparing for marketplace Windows users. Lightest fix is Option C (documentation).

Metadata

Metadata

Assignees

No one assigned

    Labels

    v1.12-backlogDeferred to v1.12+ (post-v1.12 hardening backlog)

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions