Keep bundled and reviewed-checkout skills out of Codex runs

.github/workflows/sandbox-isolation.yml

@@ -234,8 +234,8 @@ jobs:
           executed=$(grep -oE 'executed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+')
           passed=$(grep -oE 'passed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+')
           echo "executed=${executed:-0} passed=${passed:-0}"
-          if [ "${executed:-0}" -lt 108 ]; then
-            echo "::error::Expected >=108 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run re-bound at its gateway and torn down with its seal + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included + a repository whose memory links outside the workspace left out of a real Claude run, against the CLI following the link once that repository is added + nested memory loaded in place by a real Claude run, single- and multi-repo, and none of it past the in-place budget, where a read below a directory attaches only that directory's pointer + a path-scoped rule's pointer reaching a real Claude run only after a read its glob covers, single- and multi-repo, carrying none of the rule's text, against the rule copied into the config home as its control + pointers attaching on exactly the reads the unpinned CLI attached the repository's own rules on, single- and multi-repo + an Explore subagent pointed at nested memory loaded in place, which the CLI keeps out of its context + a multi-repo Codex run starting at a workspace root that is no git repository and loading no config from it + every repository's AGENTS.md reaching a real multi-repo Codex run once, after the operating contract, an override before the doc beside it and none that links outside the workspace + a repo-less Codex run starting in a scratch directory that is no git repository + a goal handed to the real Claude CLI as text it cannot act on, fresh and resumed, against the text channel as its control + a repository clean filter the capture runs with its egress severed + a real Codex agent that tampers its clone's .git while the platform publishes the branch from a clean repo, its Claude counterpart running in the non-root lane), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR."
+          if [ "${executed:-0}" -lt 111 ]; then
+            echo "::error::Expected >=111 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run re-bound at its gateway and torn down with its seal + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included + a repository whose memory links outside the workspace left out of a real Claude run, against the CLI following the link once that repository is added + nested memory loaded in place by a real Claude run, single- and multi-repo, and none of it past the in-place budget, where a read below a directory attaches only that directory's pointer + a path-scoped rule's pointer reaching a real Claude run only after a read its glob covers, single- and multi-repo, carrying none of the rule's text, against the rule copied into the config home as its control + pointers attaching on exactly the reads the unpinned CLI attached the repository's own rules on, single- and multi-repo + an Explore subagent pointed at nested memory loaded in place, which the CLI keeps out of its context + a multi-repo Codex run starting at a workspace root that is no git repository and loading no config from it + every repository's AGENTS.md reaching a real multi-repo Codex run once, after the operating contract, an override before the doc beside it and none that links outside the workspace + a repo-less Codex run starting in a scratch directory that is no git repository + a goal handed to the real Claude CLI as text it cannot act on, fresh and resumed, against the text channel as its control + a goal naming the real Codex CLI's bundled skills reaching its model with none of them loaded, fresh and resumed, against the CLI injecting every one as its control + a real Codex output reviewer loading none of the skills its reviewed checkout holds, against the CLI listing and injecting each one as its control + a repository clean filter the capture runs with its egress severed + a real Codex agent that tampers its clone's .git while the platform publishes the branch from a clean repo, its Claude counterpart running in the non-root lane), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR."
             exit 1
           fi
 
@@ -286,6 +286,18 @@ jobs:
               assert len(cases) == rows and all(r.get('outcome') == 'Passed' for r in cases), f'{method}: all {rows} case(s) must pass'
           print('All 4 goal-channel E2E arms ran and passed.')
 
+          # The Codex arms of the same E2E: a goal naming every skill the CLI bundles reaches a run that loads none of them,
+          # against a control that loads and injects every one; and an output reviewer loads none of the skills its reviewed
+          # checkout holds, against a control that lists and injects each. Each marker records the CLI it ran, which must be the pin.
+          import re
+          codex_pin = re.search(r'^ARG CODEX_CLI_VERSION=(\S+)$', open('backend/Dockerfile.worker', encoding='utf-8').read(), re.M).group(1)
+          for arm, tier in (('fresh', 'Standard'), ('resume', 'Standard'), ('review', 'Confined')):
+              assert f'[goal-channel-e2e] ran skill-mentions codex-cli {arm} {tier} uid=0 confined=True codex={codex_pin} ' in text, f'goal-channel E2E Codex arm "{arm}" did not run confined on codex {codex_pin} — check CODESPACE_REQUIRE_REVIEW_CLIS and the CLI install step'
+          for method in ('A_codex_goal_naming_the_clis_bundled_skills_reaches_the_model_without_their_bodies', 'A_continued_codex_session_resolves_none_of_the_clis_bundled_skills_either', 'A_codex_reviewer_loads_none_of_the_skills_its_reviewed_checkout_holds'):
+              cases = [r for r in results if 'GoalChannelE2ETests.' + method in r.get('testName', '')]
+              assert len(cases) == 1 and cases[0].get('outcome') == 'Passed', f'{method}: must run once and pass'
+          print('All 3 Codex goal-channel E2E arms ran and passed.')
+
           # The capture cannot switch off a repository's filter.<driver>.clean, so the only proof its run is harmless is this
           # case: a KernelFact that skips without bubblewrap, and a skip is no evidence. Require it ran once and passed.
           clean_filter = [r for r in results if 'GitWorkspaceIsolationE2ETests.A_repo_clean_filter_runs_during_capture_with_its_egress_severed' in r.get('testName', '')]

backend/src/CodeSpace.Core/Services/Agents/Harnesses/Codex/CodexHarness.cs

@@ -168,13 +168,70 @@ public sealed class CodexHarness : IAgentHarness, IAgentHarnessBinary, IAgentHar
     /// </summary>
     public const string NoProjectDocOverride = "project_doc_max_bytes=0";
 
-    /// <summary>A task that loads no repository instructions (<see cref="AgentTask.ExcludeRepositoryInstructions"/> — an output reviewer, whose cwd is the branch under review) gets <see cref="NoProjectDocOverride"/>, so the reviewed agent's own <c>AGENTS.md</c> never instructs its reviewer; every other task is byte-identical.</summary>
+    /// <summary>
+    /// The <c>-c</c> override that loads none of the skills the CLI bundles. The pinned 0.142.2 installs five of its own
+    /// under <c>CODEX_HOME/skills/.system</c> on every start (a skill installer that fetches from GitHub, a skill creator,
+    /// a plugin creator, OpenAI docs and image generation) and lists them to the model. None is a platform skill, and each
+    /// was reachable from the goal: a <c>$name</c> anywhere in it, glued to a word, after a backslash or as
+    /// <c>[$name](path)</c>, makes the CLI read that <c>SKILL.md</c> into the turn before the model acts. A goal carries
+    /// text the platform does not author: a webhook body, a fork's pull request, another model's output. The scan cannot be
+    /// avoided: <c>exec</c> hands its prompt over as one text input, the CLI scans every text input, and no setting turns
+    /// the scan off. What it can resolve can be narrowed, and this narrows it to skills the run was given. With it the CLI
+    /// lists none of its own and removes the <c>.system</c> cache from the run's config home, on <c>exec</c> and
+    /// <c>exec resume</c> alike (<c>GoalChannelE2ETests.CodexSkills</c>, measured against 0.142.2 and 0.147.0). The
+    /// persona's skills, projected under <see cref="SkillsRoot"/>, still load and still resolve a mention. So do the
+    /// repository's own, under its <c>.agents/skills</c> and <c>.codex/skills</c>, on every run that loads its
+    /// <c>AGENTS.md</c>; a run that loads neither switches them off one by one (<see cref="AppendRepositorySkillExclusion"/>).
+    /// Pinned by a test (Rule 8): the key is the CLI's, and a key the CLI does not know is ignored.
+    /// </summary>
+    public const string NoBundledSkillsOverride = "skills.bundled.enabled=false";
+
+    /// <summary>
+    /// The config key Codex reads its per-skill rules from: a list of <c>{path=…,enabled=false}</c> entries, each switching
+    /// off the skill whose <c>SKILL.md</c> resolves to that path. The CLI reads the list from the user and session-flag
+    /// layers only, so a checkout's own config cannot add to it or undo it. Pinned by a test (Rule 8): the key is the
+    /// CLI's, and a key the CLI does not know is ignored.
+    /// </summary>
+    public const string SkillRulesKey = "skills.config";
+
+    /// <summary>Every run gets <see cref="NoBundledSkillsOverride"/>, so a goal can name only the skills the platform and the repository put in front of the model.</summary>
+    private static void AppendBundledSkillExclusion(List<string> args)
+    {
+        args.Add("-c");
+        args.Add(NoBundledSkillsOverride);
+    }
+
+    /// <summary>A task that loads no repository instructions (<see cref="AgentTask.ExcludeRepositoryInstructions"/> — an output reviewer, whose cwd is the branch under review) gets <see cref="NoProjectDocOverride"/>, so the reviewed agent's own <c>AGENTS.md</c> never instructs its reviewer, and none of the checkout's skills (<see cref="AppendRepositorySkillExclusion"/>); every other task is byte-identical.</summary>
     private static void AppendRepositoryInstructionExclusion(List<string> args, AgentTask task)
     {
         if (!task.ExcludeRepositoryInstructions) return;
 
         args.Add("-c");
         args.Add(NoProjectDocOverride);
+
+        AppendRepositorySkillExclusion(args, task.WorkspaceDirectory);
+    }
+
+    /// <summary>
+    /// Switch off every skill the reviewed checkout holds, each by its path. The pinned 0.142.2 lists a checkout's skills
+    /// to the model and injects one into the turn wherever the goal names it, and a reviewer's goal holds the producer's
+    /// goal and the names of the files the producer changed: <c>notes/$approve-all.md</c> in that list put a skill the
+    /// reviewed party wrote in front of its own reviewer, as a user message, before the model acted. No setting stops the
+    /// CLI reading a checkout's skills, and only a rule per skill switches one off. A path rule, not a name rule, because a
+    /// name rule would also switch off a persona skill of the same name, which a checkout could otherwise use to take a
+    /// reviewer's persona skill away (<see cref="CodexRepositorySkills"/>, <c>GoalChannelE2ETests.CodexSkills</c>). A
+    /// checkout with no skills adds nothing.
+    /// </summary>
+    private static void AppendRepositorySkillExclusion(List<string> args, string? workspace)
+    {
+        if (string.IsNullOrWhiteSpace(workspace)) return;
+
+        var skills = CodexRepositorySkills.In(workspace);
+
+        if (skills.Count == 0) return;
+
+        args.Add("-c");
+        args.Add($"{SkillRulesKey}=[{string.Join(',', skills.Select(skill => $"{{path={McpDeclarationWriter.TomlString(skill)},enabled=false}}"))}]");
     }
 
     public SandboxSpec BuildInvocation(AgentTask task)
@@ -212,6 +269,7 @@ public SandboxSpec BuildInvocation(AgentTask task)
         AppendTelemetryConfig(args, task);
         AppendWorkspaceDistrust(args, task);
         AppendRepositoryInstructionExclusion(args, task);
+        AppendBundledSkillExclusion(args);
 
         // P3.3: Codex's default hook trust-review flow requires an interactive decision before a NON-managed command
         // hook may run — a freshly generated per-run hook has no persisted trust record, and there is no human at a