Skip to content

Keep Chromium cookie temp copies private#764

Merged
mvanhorn merged 1 commit into
mvanhorn:mainfrom
rp0927:codex/chromium-cookie-temp-perms
Jul 15, 2026
Merged

Keep Chromium cookie temp copies private#764
mvanhorn merged 1 commit into
mvanhorn:mainfrom
rp0927:codex/chromium-cookie-temp-perms

Conversation

@rp0927

@rp0927 rp0927 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace shutil.copy2() with shutil.copyfile() when copying Chromium cookie DBs to a mkstemp() path
  • keep the temp copy at 0600 immediately after content copy instead of briefly inheriting looser source DB permissions
  • add a regression test that observes the temp file mode before the existing lock-down helper runs

Validation

  • git diff --check
  • PYTHONPATH=skills/last30days/scripts uv run pytest -q tests/test_chromium_browsers.py tests/test_env_cookies.py tests/test_permission_preflight.py tests/test_project_config_trust.py tests/test_chrome_cookies.py tests/test_cookie_extract.py

Security note

copy2() calls copystat() after copying file content, which can briefly copy source permissions such as 0644 onto the temp database. Since mkstemp() creates the destination as private, copyfile() preserves the safer destination mode while still copying the DB bytes needed for SQLite reads.

@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces shutil.copy2() with shutil.copyfile() when copying the Chromium cookie database to a mkstemp() temp path, preventing copy2's implicit copystat() call from briefly pushing the source DB's looser permissions (e.g. 0644) onto the temp file before the explicit chmod 0600 runs. A POSIX-only regression test is added that spies on _lock_temp_cookie_copy to observe the temp file's mode immediately after the copy and before the explicit lock-down.

  • chrome_cookies.py: One-line change swapping copy2 for copyfile with an explanatory comment; the rest of the copy/lock/cleanup flow is unchanged.
  • test_chrome_cookies.py: New test_temp_cookie_copy_never_world_readable test creates a 0644 source DB, monkeypatches the lock helper with a spy, and asserts the temp file stays at 0600 between copy and lock.

Confidence Score: 5/5

Safe to merge — the change is minimal and targeted, replacing a single function call that was the root cause of a brief permission exposure window.

The fix is correct: copyfile() copies only bytes, leaving the 0600 mode that mkstemp() established intact until the explicit chmod runs, so there is no longer a window where the temp cookie DB is world-readable. The regression test correctly captures the mode before the lock step using a spy, and the test is correctly gated to POSIX-only. No unintended side effects were found in the surrounding copy/cleanup logic.

No files require special attention.

Important Files Changed

Filename Overview
skills/last30days/scripts/lib/chrome_cookies.py One-line security fix: swaps shutil.copy2() for shutil.copyfile() to prevent source DB permissions from briefly overriding the mkstemp() 0600 temp file before the explicit chmod runs. Comment explains the rationale clearly.
tests/test_chrome_cookies.py Adds a targeted regression test using a spy on _lock_temp_cookie_copy to assert the temp file mode is 0600 immediately after copyfile(), before the explicit lock step. The test is correctly skipped on Windows and follows existing test patterns.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant mkstemp
    participant copyfile as shutil.copyfile (new)
    participant copy2 as shutil.copy2 (removed)
    participant lock as _lock_temp_cookie_copy

    Note over Caller,lock: After fix (copyfile)
    Caller->>mkstemp: "mkstemp(suffix=.sqlite)"
    mkstemp-->>Caller: "fd, tmp_path (mode=0600)"
    Caller->>copyfile: copyfile(src, tmp_path)
    Note right of copyfile: copies bytes only, no copystat()
    copyfile-->>Caller: done (mode still 0600)
    Caller->>lock: _lock_temp_cookie_copy(tmp_path)
    lock-->>Caller: chmod 0600 (no-op)

    Note over Caller,lock: Before fix (copy2)
    Caller->>mkstemp: "mkstemp(suffix=.sqlite)"
    mkstemp-->>Caller: "fd, tmp_path (mode=0600)"
    Caller->>copy2: copy2(src, tmp_path)
    Note right of copy2: copies bytes then copystat() mode briefly 0644
    copy2-->>Caller: "done (mode=0644 EXPOSED)"
    Caller->>lock: _lock_temp_cookie_copy(tmp_path)
    lock-->>Caller: chmod 0600 (closes window)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant mkstemp
    participant copyfile as shutil.copyfile (new)
    participant copy2 as shutil.copy2 (removed)
    participant lock as _lock_temp_cookie_copy

    Note over Caller,lock: After fix (copyfile)
    Caller->>mkstemp: "mkstemp(suffix=.sqlite)"
    mkstemp-->>Caller: "fd, tmp_path (mode=0600)"
    Caller->>copyfile: copyfile(src, tmp_path)
    Note right of copyfile: copies bytes only, no copystat()
    copyfile-->>Caller: done (mode still 0600)
    Caller->>lock: _lock_temp_cookie_copy(tmp_path)
    lock-->>Caller: chmod 0600 (no-op)

    Note over Caller,lock: Before fix (copy2)
    Caller->>mkstemp: "mkstemp(suffix=.sqlite)"
    mkstemp-->>Caller: "fd, tmp_path (mode=0600)"
    Caller->>copy2: copy2(src, tmp_path)
    Note right of copy2: copies bytes then copystat() mode briefly 0644
    copy2-->>Caller: "done (mode=0644 EXPOSED)"
    Caller->>lock: _lock_temp_cookie_copy(tmp_path)
    lock-->>Caller: chmod 0600 (closes window)
Loading

Reviews (1): Last reviewed commit: "fix: keep Chromium cookie temp copies pr..." | Re-trigger Greptile

@mvanhorn
mvanhorn merged commit 4488271 into mvanhorn:main Jul 15, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants