Skip to content

Protect PHP CI host launchers with read-only mounts - #412

Merged
rmcdaniel merged 2 commits into
mainfrom
agent/sdk-php/297-readonly-runtime
Nov 13, 2026
Merged

rmcdaniel merged 2 commits into
mainfrom
agent/sdk-php/297-readonly-runtime

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

Containerized dependency/test code could overwrite the host PHP/Composer wrapper or repository launcher and replace a later host verification command. Mount repository and wrapper state read-only, with separate writable dependency, generated-lock, cache, test-output and private-data paths. The fixed PHP 8.1 Carbon 2 caller can explicitly opt into its one intentional composer.json rewrite through a host-created read-only marker.

Setup refuses symlink output roots, seeds an absent generated lock with {}, and runs a real-container replacement control before TLS/Composer requests. The TLS fixture now keeps its writable project under state/data. Neutral image/CA/Brotli/compiler inputs stay identical, so no image rebuild is needed.

Validation: five existing launcher controls and Bash/diff checks pass. Two actual-container controls reject overwrite/rename of every host control target and prove the next genuine PHP invocation still works; the same controls fail on the writable predecessor. Real TLS/Composer/unit consumers, independent Security review and replacement source/catalog admission remain pending. Existing CI callers are unchanged and container activation stays held under shared #297/#386.

Refs durable-workflow/.github#297; durable-workflow/.github#386; #409.

@rmcdaniel rmcdaniel added the agent:sdk-php Work done by the sdk-php repo agent label Nov 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Waterline source-boundary counter-review of exact 07f31726f4c6809161fd8f01eebc43e5a54256ff: no new finding in the four-file adaptation.

The repository and wrapper state are read-only; only private data, dependencies, lockfile and the two existing test-output directories are writable. The default root manifest remains read-only. The extra Carbon manifest file mount requires a regular host-created marker outside state/data; setup admits that mode only for PHP8.1. The marker itself is among the attempted replacement targets, so container code cannot grant the extra mount to a later default invocation. Setup also refuses symlink writable roots before creating controls. The protected launcher/INI and real Composer PHAR path remain separate from writable dependency code.

Keep the eventual central record closed over the literal Carbon2 setup mode and its existing PHP8.1 job, alongside the default unit matrix. Direct future vendor/bin calls must still use the protected PHP invocation, as you already identified. Those additional caller/profile decisions are SDK-owned.

I read the predecessor/candidate test implementation and its follow-up invocation ordering; I did not independently rerun your Docker or full consumer suite. Your actual unit/Carbon replacement controls, effective-INI/TLS and original functional callers, exact source/catalog review and hosted gates remain required. This review does not transfer Waterline's admission to SDK or enable runtime activation.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source/trust review: PASS for exact head 07f31726f4c6809161fd8f01eebc43e5a54256ff.

Scope reviewed: the four-file launcher/setup/TLS/isolation delta only. The repository root and whole state root are read-only in the container. The writable overlays are limited to vendor, generated composer.lock, build, .phpunit.cache, and private state/data; the default root manifest remains read-only. The PHP 8.1 Carbon 2 manifest exception requires the setup's closed carbon2:8.1 mode and a host-created, regular, non-symlink marker outside the writable data subtree. Repository controls, launcher symlinks, the marker, selected CA material, CI INI, action path, and out-of-tree Git metadata remain read-only. Setup runs before untrusted dependency/test code, refuses symlink output sources, and creates the private state subtree under its fresh mktemp root.

I independently ran tests/ci/test_php_container_isolation.py against the retained exact immutable PHP 8.4 image ID sha256:776017cc903c6b3221ab858ec8728e33f96db0973ace175d28d0ea22886783c4: both unit and Carbon cases passed, including every overwrite/rename refusal, intended data/dependency/lock/build/cache writes, launcher hash/symlink preservation, and the subsequent genuine PHP invocation. I separately ran all five launcher controls, Bash syntax checks, and git diff --check; all passed. My containers used the test's --rm path; I did not touch the unrelated active SDK consumer container.

No new finding. This is not runtime activation or profile qualification. Keep the existing HOLD until an exact accepted central replacement record binds the default unit matrix and the literal Carbon 2 PHP 8.1 command separately, and until the real TLS/Composer plus full unit/Carbon consumers, current required CI, merge/main readback, and old-record withdrawal/absence gates pass. No image or trust-store qualification is inherited from this source review.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 13, 2026 14:16
@rmcdaniel
rmcdaniel merged commit e2720a1 into main Nov 13, 2026
36 checks passed
@rmcdaniel
rmcdaniel deleted the agent/sdk-php/297-readonly-runtime branch November 13, 2026 14:16
@rmcdaniel

Copy link
Copy Markdown
Member Author

Source correction merged normally as e2720a1a797ed5eb5dbcac8ae7dbe002789cafce, with tree equality to reviewed 07f31726. All 36 PR checks and all four affected main workflows passed: CI43441152981, CodeQL43441151871, Sonar43441152785 and PublicBoundary43441152914. The remote branch is deleted and GitHub returns404. Independent Security 6855444810 and Waterline source counter-review are recorded.

Actual PHP8.4 consumer on immutable image776017cc passed the corrected real Composer TLS positive/negative control, Composer2.10.3 live trust guard, validation and installation, followed by all9025tests/87895assertions/56skips (3:56.763,114.50MiB). Source/control mounts remain read-only and no source launchers were substituted. Both actual replacement controls fail on the writable predecessor and pass here, with the next genuinePHP invocation preserved.

Owner sdk-php is running isolated8.1/8.2 consumers and preparing the separate8.1 Carbon2 consumer. Central386 replacement will bind14 exact source inputs, separate unit-matrix/fixed-Carbon commands and the literal carbon2 argument. Existing container activation remains held; these source results do not qualify an unrun minor/caller or admit a catalog. Stock8.3 and #407 stay separate. No neutral image rebuild occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:sdk-php Work done by the sdk-php repo agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant