Skip to content

Fix Linux bundled plugin cache permissions#1026

Closed
psypeal wants to merge 1 commit into
ilysenko:mainfrom
psypeal:codex/fix-chrome-cache-permissions
Closed

Fix Linux bundled plugin cache permissions#1026
psypeal wants to merge 1 commit into
ilysenko:mainfrom
psypeal:codex/fix-chrome-cache-permissions

Conversation

@psypeal

@psypeal psypeal commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the recurring Linux Chrome side-panel failure at the shared bundled-plugin materialization boundary.

This is a focused follow-up to #962. It does not change the launcher, Chrome extension, or native-host trust validation.

Root cause

The launcher can start with a trusted bundled Chrome cache, but Electron later rematerializes that marketplace under a cooperative umask 0002. The previous implementation normalized the copied tree after the copy, which could accept an already unsafe or tampered source and then make it appear trusted.

The native host correctly rejects a group/world-writable browser-client.mjs or parent chain, producing Codex app-server manifest entry is missing required path parent after a rebuild, reinstall, or later marketplace refresh.

Review update

The implementation now enforces trust before materialization:

  • Validate the canonical bundled-plugin source, every relevant ancestor, and the complete source tree before copying.
  • Reject symlinks, non-file/non-directory entries, unexpected ownership, and group/world-writable entries. Root-owned sticky directories remain valid ancestors.
  • Create the Linux marketplace staging root recursively with mode 0700 before any plugin copy.
  • Copy only a source that passed validation, then add only the owner-write bit needed by the marketplace installer.
  • Fail closed when source validation fails; an unsafe browser-client.mjs is never copied.

This removes the earlier post-copy & ~0o022 approach that could "bless" an unsafe source.

Regression coverage

  • A trusted read-only plugin is copied through a newly created private 0700 staging root and remains owner-writable afterward.
  • A tampered browser-client.mjs with mode 0664 is rejected before materialization, and the target is not created.
  • The patch remains idempotent against the current upstream minified bundle shape.

Validation

  • node --test scripts/patch-linux-window-ui.test.js — 398/398 passed.
  • node --test scripts/patches/impl/main-process/browser.test.js — 5/5 passed.
  • Current-DMG exact main-bundle probe — patch applied once and remained idempotent.
  • bash -n launcher/start.sh.template scripts/lib/*.sh — passed.
  • node --check scripts/patches/impl/main-process/browser.js — passed.
  • git diff --check — passed.
  • bash tests/scripts_smoke.sh — the affected Chrome cache-permission probe passed. The suite later stopped at an unrelated host limitation: this Python lacks os.pidfd_open and os.pidfd_send_signal, so the warm-start test refused to terminate a stale Electron process.
  • Two independent local read-only reviews were run. The first found missing-parent staging creation, which was fixed and regression-tested; the second found no actionable defect.

Scope and risk

Only the existing shared Linux bundled-plugin materializer patch and its regression tests change (two files). There are no launcher, native-host, extension, packaging, or generated-output changes.

The separate stale local development manifest entry found during live diagnosis was repaired only in local runtime state and is intentionally not part of this PR.

@psypeal
psypeal marked this pull request as ready for review July 16, 2026 12:07
@ilysenko

Copy link
Copy Markdown
Owner

Please try to limit this. Please do not create a second pull request while the first one is still open. There is no need to create multiple PRs. We do not have enough GitHub Actions capacity.

@psypeal

psypeal commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Please try to limit this. Please do not create a second pull request while the first one is still open. There is no need to create multiple PRs. We do not have enough GitHub Actions capacity.

oh, my bad. I will pause it.

@ilysenko ilysenko closed this Jul 16, 2026
@ilysenko ilysenko reopened this Jul 16, 2026
@ilysenko ilysenko closed this Jul 16, 2026
@ilysenko ilysenko reopened this Jul 16, 2026

@ilysenko ilysenko left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused fix. The shared Linux bundled-plugin materializer is the correct ownership point for this invariant; the fix should not be moved to the launcher or the native host.

The blocker is how the invariant is enforced. The new post-copy & ~0o022 normalization accepts a group/world-writable source tree and makes its copied contents pass the native-host trust checks. This can bless executable content modified before or during the copy, and the updated test currently locks in exactly that unsafe 0775/0664 source shape. This conflicts with the trust model established in #852 and #962.

Please keep the fix in this materialization path, but enforce trust at the creation boundary: validate the source and relevant ancestors before copying, then rebuild from verified bundled resources through private staging or fail closed. Do not make an unsafe existing tree trusted with chmod. Please also add a regression proving that a tampered browser-client.mjs is never accepted as trusted.

Please verify the complete update against CONTRIBUTING.md, run the relevant tests locally before pushing it, perform an independent full-diff review with a local LLM before requesting maintainer review, and wait for every required CI check to pass after the update.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for contributing. This repository allows a maximum of 1 active pull request per contributor. You currently have 2 open pull requests, so this pull request is being closed automatically. Please finish or close one of your existing pull requests before opening another.

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