Skip to content

feat: add readOnly field on workspace sources for immutable file mounts#241

Open
xcoulon wants to merge 1 commit into
codeready-toolchain:masterfrom
xcoulon:agent-sources-readonly
Open

feat: add readOnly field on workspace sources for immutable file mounts#241
xcoulon wants to merge 1 commit into
codeready-toolchain:masterfrom
xcoulon:agent-sources-readonly

Conversation

@xcoulon

@xcoulon xcoulon commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Add a readOnly boolean field to InlineSource, ConfigMapSource, and
GitSource. When set, items bypass PVC seeding and are mounted directly
into the gateway container's workspace path via Kubernetes read-only
volume mounts, enforcing OS-level EROFS protection.

  • generateSeedManifest() skips entries from read-only sources
  • New injectReadOnlyWorkspaceMounts() adds per-item subPath mounts
    on the gateway container from existing volumes (config, ws-cm-*,
    ws-git-*)
  • Unit tests cover all three source types, mixed read-only/writable
    specs, and correct git source indexing
  • Integration tests (envtest) verify deployment volumeMounts and seed
    manifest exclusion after full reconciliation
  • E2e tests verify runtime immutability via kubectl exec write
    attempts returning EROFS

Assisted-by: Claude Opus 4.6 (1M context)
Signed-off-by: Xavier Coulon xcoulon@redhat.com

Summary by CodeRabbit

  • New Features

    • Added source-level read-only workspace files for inline, ConfigMap, and Git sources.
    • Read-only files are mounted directly into the gateway workspace and cannot be modified or deleted at runtime.
    • Writable sources continue to be seeded normally, allowing read-only and writable files to coexist.
  • Documentation

    • Clarified that seeding modes are ignored when read-only mode is enabled.

…unts

Add a `readOnly` boolean field to `InlineSource`, `ConfigMapSource`, and
`GitSource`. When set, items bypass PVC seeding and are mounted directly
into the gateway container's workspace path via Kubernetes read-only
volume mounts, enforcing OS-level EROFS protection.

- `generateSeedManifest()` skips entries from read-only sources
- New `injectReadOnlyWorkspaceMounts()` adds per-item `subPath` mounts
  on the gateway container from existing volumes (`config`, `ws-cm-*`,
  `ws-git-*`)
- Unit tests cover all three source types, mixed read-only/writable
  specs, and correct git source indexing
- Integration tests (envtest) verify deployment volumeMounts and seed
  manifest exclusion after full reconciliation
- E2e tests verify runtime immutability via `kubectl exec` write
  attempts returning EROFS

Assisted-by: Claude Opus 4.6 (1M context)
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Walkthrough

Adds source-level readOnly support for inline, ConfigMap, and Git workspace sources. Read-only items bypass seed-manifest/PVC seeding and mount directly into the gateway container through Kubernetes read-only volume mounts, with controller, integration, specification, and e2e coverage.

Changes

Read-only workspace sources

Layer / File(s) Summary
Source contract and documented behavior
api/v1alpha1/claw_types.go, config/crd/bases/..., openspec/...
Adds readOnly to all workspace source types and documents ignored mode values, direct mounts, seed exclusion, and expected mixed-source behavior.
Seed filtering and gateway mount injection
internal/controller/claw_workspace.go, internal/controller/claw_resource_controller.go
Excludes read-only items from seed manifests and injects source-backed read-only volumeMounts into the gateway deployment.
Controller behavior validation
internal/controller/claw_workspace_test.go
Tests source-specific mounts, seed filtering, mixed writable/read-only inputs, and reconcile output.
End-to-end filesystem verification
test/e2e/e2e_test.go
Verifies read-only inline, ConfigMap, and Git files, writable mixed-source files, and resulting seed manifests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClawSpec
  participant Controller
  participant SeedManifest
  participant GatewayDeployment
  ClawSpec->>Controller: configure workspace sources
  Controller->>SeedManifest: omit read-only items
  Controller->>GatewayDeployment: add read-only volumeMounts
  GatewayDeployment-->>Controller: render updated workload
Loading

Possibly related PRs

Suggested labels: feature, test

Suggested reviewers: alexeykazakov, pavelanni

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a readOnly field to workspace sources for immutable file mounts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added feature New feature or request test Work that adds, fixes, or maintains automated tests or coverage (unit, integration, e2e, flakiness) labels Jul 15, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.71014% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.63%. Comparing base (88f3b52) to head (8700166).

Files with missing lines Patch % Lines
internal/controller/claw_workspace.go 82.08% 7 Missing and 5 partials ⚠️
internal/controller/claw_resource_controller.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #241   +/-   ##
=======================================
  Coverage   79.63%   79.63%           
=======================================
  Files          34       34           
  Lines        4900     4969   +69     
=======================================
+ Hits         3902     3957   +55     
- Misses        634      642    +8     
- Partials      364      370    +6     
Files with missing lines Coverage Δ
internal/controller/claw_resource_controller.go 63.68% <0.00%> (-0.14%) ⬇️
internal/controller/claw_workspace.go 89.64% <82.08%> (-1.25%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
internal/controller/claw_workspace_test.go (1)

594-682: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Convert the repeated source cases to table-driven tests.

Use case tables containing the workspace spec, fixtures, expected manifest targets, and expected mounts. This removes substantial repeated setup and keeps Inline, ConfigMap, and Git expectations aligned.

As per coding guidelines, **/*_test.go: “Structure tests with Test* function names, use t.Run() for subtests, t.Cleanup() for cleanup, and implement table-driven test patterns.”

Also applies to: 1673-1876, 2274-2598

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/controller/claw_workspace_test.go` around lines 594 - 682, The
repeated read-only source subtests around generateSeedManifest, plus the
similarly structured cases in the referenced test sections, should be
consolidated into table-driven tests. Define cases containing the WorkspaceSpec,
source fixtures, expected manifest targets, and expected mounts, then iterate
with t.Run to assert each case; keep InlineSources, ConfigMapSources, and
GitSources expectations aligned while removing duplicated setup and assertions.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md`:
- Around line 3-10: Clarify the guarantee in the proposal’s readOnly behavior:
either limit it to protecting the mounted file or define and implement
immutability for the entire target-path chain, including writable ancestors. In
the workspace-sources spec scenario at
openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md:25-28,
add coverage for renaming a writable ancestor and recreating the original path;
update the scenario to match the chosen guarantee.

In `@openspec/specs/workspace-sources/spec.md`:
- Around line 156-160: Update the “Read-only inline source is mounted directly”
scenario to use the correct encoded subPath `_ws_POLICY.md` for the slash-free
`POLICY.md` path, while retaining the existing `--` encoding example for paths
containing directory separators.

---

Nitpick comments:
In `@internal/controller/claw_workspace_test.go`:
- Around line 594-682: The repeated read-only source subtests around
generateSeedManifest, plus the similarly structured cases in the referenced test
sections, should be consolidated into table-driven tests. Define cases
containing the WorkspaceSpec, source fixtures, expected manifest targets, and
expected mounts, then iterate with t.Run to assert each case; keep
InlineSources, ConfigMapSources, and GitSources expectations aligned while
removing duplicated setup and assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: cfc71c4d-4ecf-416d-b6a6-1d7d994cc766

📥 Commits

Reviewing files that changed from the base of the PR and between 88f3b52 and 8700166.

📒 Files selected for processing (11)
  • api/v1alpha1/claw_types.go
  • config/crd/bases/claw.sandbox.redhat.com_claws.yaml
  • internal/controller/claw_resource_controller.go
  • internal/controller/claw_workspace.go
  • internal/controller/claw_workspace_test.go
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/design.md
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/tasks.md
  • openspec/specs/workspace-sources/spec.md
  • test/e2e/e2e_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • codeready-toolchain/api (manual)
  • codeready-toolchain/toolchain-common (manual)
  • codeready-toolchain/host-operator (manual)
  • codeready-toolchain/toolchain-e2e (manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Lint
  • GitHub Check: E2E Tests
  • GitHub Check: Unit Tests
🧰 Additional context used
📓 Path-based instructions (6)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/tasks.md
  • internal/controller/claw_resource_controller.go
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md
  • config/crd/bases/claw.sandbox.redhat.com_claws.yaml
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/design.md
  • openspec/specs/workspace-sources/spec.md
  • api/v1alpha1/claw_types.go
  • test/e2e/e2e_test.go
  • internal/controller/claw_workspace.go
  • internal/controller/claw_workspace_test.go
internal/controller/**/*_controller.go

📄 CodeRabbit inference engine (CLAUDE.md)

Apply RBAC authorization via // +kubebuilder:rbac:... markers on reconciler methods in the controller

Files:

  • internal/controller/claw_resource_controller.go
internal/controller/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Set owner references on all created resources via controllerutil.SetControllerReference

Files:

  • internal/controller/claw_resource_controller.go
  • internal/controller/claw_workspace.go
  • internal/controller/claw_workspace_test.go
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Include Go license header from hack/boilerplate.go.txt template in all Go source files
Run golangci-lint via make lint and use make lint-fix for auto-fixing, with .golangci.yml configuration including lll and dupl linters enabled
Use make fmt (go fmt) and make vet (go vet) for code formatting and vetting before committing

Files:

  • internal/controller/claw_resource_controller.go
  • api/v1alpha1/claw_types.go
  • test/e2e/e2e_test.go
  • internal/controller/claw_workspace.go
  • internal/controller/claw_workspace_test.go
api/v1alpha1/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Modify API types in api/v1alpha1/ and run make manifests to regenerate CRD YAML in config/crd/bases/ and make generate to regenerate zz_generated.deepcopy.go

Files:

  • api/v1alpha1/claw_types.go
**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*_test.go: Use require assertion from testify for fatal setup errors in tests, and assert for value comparisons
Structure tests with Test* function names, use t.Run() for subtests, t.Cleanup() for cleanup, and implement table-driven test patterns
Use waitFor(t, timeout, interval, condition, message) helper for async test assertions with 10s timeout and 250ms poll interval
Create separate test files per resource type (e.g., claw_configmap_test.go, claw_credentials_test.go)

Files:

  • test/e2e/e2e_test.go
  • internal/controller/claw_workspace_test.go
🪛 LanguageTool
openspec/changes/archive/2026-07-15-read-only-workspace-sources/design.md

[style] ~94-~94: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ubernetes handles many mounts well, but very large numbers (100+) could affect pod startup...

(EN_WEAK_ADJECTIVE)

🔇 Additional comments (11)
api/v1alpha1/claw_types.go (1)

411-419: LGTM!

Also applies to: 460-468, 513-521

config/crd/bases/claw.sandbox.redhat.com_claws.yaml (1)

904-914: LGTM!

Also applies to: 967-977, 1026-1028, 1038-1043

internal/controller/claw_workspace.go (2)

45-45: LGTM!

Also applies to: 229-257


722-814: 🎯 Functional Correctness

No change needed.

test/e2e/e2e_test.go (1)

111-127: LGTM!

Also applies to: 2349-2452, 2453-2554, 2555-2636

openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md (1)

1-2: LGTM!

Also applies to: 11-26

openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md (1)

1-24: LGTM!

Also applies to: 30-49

openspec/changes/archive/2026-07-15-read-only-workspace-sources/tasks.md (2)

30-34: 📐 Maintainability & Code Quality

Confirm the required formatting and vet checks.

Please run and record make fmt and make vet; only build, lint, and test are listed.

As per coding guidelines, **/*.go: “Use make fmt (go fmt) and make vet (go vet) for code formatting and vetting before committing.”

Source: Coding guidelines


1-29: LGTM!

internal/controller/claw_resource_controller.go (1)

889-891: LGTM!

internal/controller/claw_workspace_test.go (1)

27-27: LGTM!

Comment on lines +3 to +10
Workspace sources (inline, ConfigMap, Git) currently seed files onto the PVC, where the OpenClaw process can freely modify or delete them at runtime. For managed reference files (company policies, compliance docs, skill definitions, curated configs), the operator needs a way to guarantee the files remain untouched — preventing both accidental and intentional edits by the agent.

The existing `mode: overwrite` re-seeds on restart but does not protect files during runtime.

## What Changes

- Add a `readOnly` boolean field to `InlineSource`, `ConfigMapSource`, and `GitSource`
- When `readOnly: true`, bypass PVC seeding entirely and mount source files directly into the gateway container's workspace path using Kubernetes read-only volume mounts (OS-level enforcement via `EROFS`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## Proposal\n'
sed -n '1,120p' openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md

printf '\n## Spec\n'
sed -n '1,120p' openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md

printf '\n## Search for readOnly / mount wording\n'
rg -n "readOnly|EROFS|mount.*workspace|workspace path|inline, ConfigMap, Git" openspec -S

Repository: codeready-toolchain/claw-operator

Length of output: 17261


🏁 Script executed (no clone):

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
print("noop")
PY

Length of output: 130


🌐 Web query:

Linux rename writable parent directory mountpoint path replace read-only bind mount EROFS ancestor rename semantics

💡 Result:

In Linux, the rename(2) system call operates on directory entries within a filesystem [1]. The ability to perform a rename operation is fundamentally governed by the write permissions of the parent directories involved and the read-only status of the mount points containing those entries [2][3]. 1. Mount Point Semantics: A rename operation requires the ability to modify the directory entries of both the source and destination parent directories [1]. If a parent directory is located on a read-only bind mount (or any read-only mount), the kernel will return EROFS (Read-only file system) because the operation requires metadata modification (i.e., unlinking the source and creating/overwriting the destination) [4][2]. This is consistent with the VFS design where read-only mounts prevent any filesystem state mutation [2]. 2. EROFS and Ancestry: The EROFS error is triggered when the kernel's VFS layer detects that the mount point covering the parent directory has its read-only flag set [2]. This check occurs during the lookup and permission-validation phase of the system call [3]. Renaming is restricted to within a single mount point (or, in some experimental/specific contexts, within the same superblock, though standard behavior enforces mount-point isolation) [5][6]. Because a rename involves unlinking from the source parent and linking in the destination parent, if either parent resides on a read-only mount, the operation will fail with EROFS [4][2]. 3. Locking Semantics: The kernel uses a strict locking hierarchy for rename operations to prevent deadlocks, particularly during cross-directory renames [7][8]. The kernel locks the filesystem's rename mutex (s_vfs_rename_mutex) and then acquires exclusive locks (i_rwsem) on the parent directories and relevant inodes [7][8]. If the operation is attempted on a path that traverses a read-only mount point, the permission check (typically performed via permission or mnt_want_write helpers) will fail before the rename can proceed, returning EROFS [4][2][3]. In summary, you cannot rename a directory (or any file) if the parent directory is part of a read-only mount, as the VFS layer requires write access to the mount point to perform the necessary inode and directory entry updates [2][3]. Attempts to do so will consistently yield EROFS [4][2].

Citations:


🌐 Web query:

man renameat mount point rename directory containing mountpoint writable ancestor

💡 Result:

The renameat system call, like the related rename call, generally cannot be used to rename a directory if it is currently in use as a mount point [1][2][3]. When a directory is used as a mount point, attempts to rename it typically fail with an EBUSY error [1][3]. The EBUSY error code signifies that the resource is currently in use by the system [1]. While some documentation notes that there is no strict technical requirement to return EBUSY in all such cases—as it is sometimes possible for the system to handle the rename—it is explicitly permitted and common for systems to return EBUSY when a directory is being used as a mount point [4][1]. If you need to rename a mount point, the standard and safe approach is to: 1. Unmount the filesystem currently using the directory as its mount point [5][6][3]. 2. Rename the directory using the renameat or rename system calls (or standard command-line tools like mv) [5][3]. 3. Update your mount configuration (such as /etc/fstab) to reflect the new path [5][6][3]. 4. Remount the filesystem at the new location [6][3]. Alternatively, you can create a new directory, mount the desired filesystem there, and then unmount the original location if necessary [6][3]. It is also possible in some environments to use bind mounts to redirect traffic to a new location without unmounting the source, though this does not rename the original directory itself [3].

Citations:


🏁 Script executed (no clone):

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import urllib.request
urls = [
    "https://man7.org/linux/man-pages/man2/renameat.2.html",
    "https://man7.org/linux/man-pages/man8/mount.8.html",
]
for u in urls:
    print(f"\n### {u}")
    try:
        with urllib.request.urlopen(u, timeout=20) as r:
            text = r.read().decode("utf-8", "replace")
        for needle in ["mount point", "busy", "EXDEV", "EROFS", "rename"]:
            if needle.lower() in text.lower():
                idx = text.lower().find(needle.lower())
                print(text[max(0, idx-300): idx+500].replace("\n", " "))
                break
    except Exception as e:
        print("ERR", e)
PY

Length of output: 516


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "InlineSource|ConfigMapSource|GitSource|workspace path|readOnly" openspec/changes/archive/2026-07-15-read-only-workspace-sources -S

Repository: codeready-toolchain/claw-operator

Length of output: 8261


🌐 Web query:

Linux rename directory containing nested bind mount allowed mountpoint inside directory rename ancestor path replacement

💡 Result:

In Linux, you generally cannot rename a directory that is a mount point, nor can you perform a rename operation that crosses a mount point boundary (including bind mounts), even if the source and destination are on the same underlying filesystem [1][2][3]. The rename(2) system call is explicitly designed to prevent crossing mount boundaries, which acts as a security and isolation boundary [1][2]. When you attempt to rename a path across a mount point, the kernel returns an EXDEV error (Invalid cross-device link) [4][1][5]. This limitation is deliberate and persists regardless of whether the mount points share the same underlying superblock [1][2]. Key Technical Points: 1. Cannot Rename Mount Points: A directory that serves as a mount point cannot be renamed [6][7]. To change the name of a mount point, you must unmount the filesystem (or bind mount), rename the directory, and then remount it at the new location [6][7]. 2. Rename Across Bind Mounts: If you have a bind mount inside a directory, or if the source and destination of a move/rename are separated by a bind mount, standard rename operations will fail with EXDEV [4][1][5]. Tools like mv(1) will detect this error and typically fall back to a manual copy-and-delete operation [4][5]. 3. Kernel Limitations vs. Userspace: While modern efforts (such as proposed kernel patches or recent coreutils optimizations) seek to optimize or allow these operations when the source and destination share the same underlying device/superblock, the fundamental restriction on crossing mount point boundaries remains a core feature of the Linux Virtual File System (VFS) to maintain strict tree boundaries [8][9][2]. 4. Alternative for Moving Mounts: If you need to move an entire mount point (not rename the directory name, but move the mount object itself), Linux provides the move_mount(2) system call, which allows you to move an existing mount object to a new location in the mount namespace without unmounting it first [10]. In summary, you cannot rename a directory if it contains a mount point or is itself a mount point using standard file manipulation commands; you must unmount the nested mount points, perform the rename, and remount the filesystem [6][7].

Citations:


🌐 Web query:

can you rename a directory that contains a bind mount child linux

💡 Result:

Yes, you can rename a parent directory that contains a bind-mounted child directory [1]. Renaming the parent directory does not invalidate or break the bind mount itself [2]. When you rename a directory containing a bind mount: 1. The bind mount persists at its original absolute path in the system's mount table [2]. 2. Any process accessing the bind-mounted directory via its original path will continue to see the mounted content [2]. 3. The directory entry for the bind mount point itself remains unchanged, even though its parent's name has changed [2]. Important limitations regarding the rename(2) system call: While the OS allows you to rename the parent directory, you generally cannot rename or move the mount point directory itself [1]. Furthermore, because Linux treats different mount points as distinct filesystem boundaries, attempting to move files or directories across these boundaries using standard tools like mv often triggers an EXDEV error (invalid cross-device link) [3][4][5]. In such cases, tools like mv typically fall back to a copy-and-delete operation rather than a simple metadata rename [3][6]. Recent improvements in coreutils (such as optimizations to detect when source and destination share the same underlying device despite different mount points) are helping to make these operations more efficient [6][7].

Citations:


Read-only file mounts don’t make the workspace path immutable

A readOnly: true mount only protects the mounted file. If the workspace directory stays writable, it can be renamed and the original path recreated, so EROFS on the file itself doesn’t block path shadowing.

  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md#L3-L10: either narrow the guarantee to the mounted file or make the whole target-path chain immutable.
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md#L25-L28: add a scenario that renames a writable ancestor and recreates the original path.
📍 Affects 2 files
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md#L3-L10 (this comment)
  • openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md#L25-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openspec/changes/archive/2026-07-15-read-only-workspace-sources/proposal.md`
around lines 3 - 10, Clarify the guarantee in the proposal’s readOnly behavior:
either limit it to protecting the mounted file or define and implement
immutability for the entire target-path chain, including writable ancestors. In
the workspace-sources spec scenario at
openspec/changes/archive/2026-07-15-read-only-workspace-sources/specs/workspace-sources/spec.md:25-28,
add coverage for renaming a writable ancestor and recreating the original path;
update the scenario to match the chosen guarantee.

Comment on lines +156 to +160
#### Scenario: Read-only inline source is mounted directly
- **GIVEN** a Claw CR with an inline source `{path: "POLICY.md", content: "Do not delete", readOnly: true}`
- **WHEN** the controller reconciles
- **THEN** the gateway container SHALL have a volumeMount from the `config` volume at `/home/node/.openclaw/workspace/POLICY.md` with `subPath: _ws_POLICY--md` (or equivalent encoded path) and `readOnly: true`
- **AND** the seed manifest SHALL NOT contain an entry for `POLICY.md`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Incorrect example subPath for a path with no slash.

For path: "POLICY.md" (no /), encodeWorkspacePath only replaces / characters, so the real subPath is _ws_POLICY.md, not _ws_POLICY--md. The -- encoding only appears when the path contains a directory separator (as correctly shown elsewhere for docs/POLICY.md_ws_docs--POLICY.md).

📝 Proposed fix
-- **THEN** the gateway container SHALL have a volumeMount from the `config` volume at `/home/node/.openclaw/workspace/POLICY.md` with `subPath: _ws_POLICY--md` (or equivalent encoded path) and `readOnly: true`
+- **THEN** the gateway container SHALL have a volumeMount from the `config` volume at `/home/node/.openclaw/workspace/POLICY.md` with `subPath: _ws_POLICY.md` (encoded path; `/` in the workspace path becomes `--`) and `readOnly: true`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### Scenario: Read-only inline source is mounted directly
- **GIVEN** a Claw CR with an inline source `{path: "POLICY.md", content: "Do not delete", readOnly: true}`
- **WHEN** the controller reconciles
- **THEN** the gateway container SHALL have a volumeMount from the `config` volume at `/home/node/.openclaw/workspace/POLICY.md` with `subPath: _ws_POLICY--md` (or equivalent encoded path) and `readOnly: true`
- **AND** the seed manifest SHALL NOT contain an entry for `POLICY.md`
#### Scenario: Read-only inline source is mounted directly
- **GIVEN** a Claw CR with an inline source `{path: "POLICY.md", content: "Do not delete", readOnly: true}`
- **WHEN** the controller reconciles
- **THEN** the gateway container SHALL have a volumeMount from the `config` volume at `/home/node/.openclaw/workspace/POLICY.md` with `subPath: _ws_POLICY.md` (encoded path; `/` in the workspace path becomes `--`) and `readOnly: true`
- **AND** the seed manifest SHALL NOT contain an entry for `POLICY.md`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openspec/specs/workspace-sources/spec.md` around lines 156 - 160, Update the
“Read-only inline source is mounted directly” scenario to use the correct
encoded subPath `_ws_POLICY.md` for the slash-free `POLICY.md` path, while
retaining the existing `--` encoding example for paths containing directory
separators.

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

Labels

feature New feature or request test Work that adds, fixes, or maintains automated tests or coverage (unit, integration, e2e, flakiness)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants