An artifact directory outside the build's workspace is refused (v1.43.7) #167

2026-10-24T19:34:53+02:00

claude-code commented

2026-10-24 19:34:53 +02:00

See docs/prs/2026-10-24-PR#167-artifact-dirs-inside-workspace.md

🤖 Generated with Claude Code

See docs/prs/2026-10-24-PR#167-artifact-dirs-inside-workspace.md 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-code added 1 commit 2026-10-24 19:34:53 +02:00

PR-doc: an artifact directory outside the build's workspace is refused

werkator build succeeded after 02:46

Details

818a24d81e

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-code added 1 commit 2026-10-24 19:39:37 +02:00

An artifact directory outside the build's workspace is refused (v1.43.7)

werkator build succeeded after 02:29

Details

621ea2b8d0

artifactDirs is a branch setting, and the server copied each entry on the host,
outside the sandbox: an absolute path or '..' made it copy any host directory,
e.g. the machine config with the Gitea token, into the build's artifacts.
ConfigLoader now refuses such an entry, and FileArtifactStore checks again
before the copy, with the real path, so a symlink in the workspace cannot escape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-code added 1 commit 2026-10-24 19:42:52 +02:00

PR-doc names the check as the code does

werkator build succeeded after 02:12

Details

ai-review/assessment AI findings; a developer approval after checking them unlocks the merge; review requested from mi; merge scheduled for when the checks succeed

Details

ai-review/gate build succeeded after 07:23

Details

ai-review/codex author ✍️: resolve 1 open Codex finding(s)

e4cccbd231

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-code marked the pull request as ready for review 2026-10-24 19:45:39 +02:00

ai-reviewer commented

2026-10-24 19:53:04 +02:00

🟠 werkrevisor ORANGE for e4cccbd, superseded by the result for 5c6a245

🟠 werkrevisor: ORANGE / FINDINGS

Change kind: mixed.

Reasons

ORANGE 1 finding(s) that Jev rates as probably real and severe: src/main/kotlin/de/javagil/werkator/artifacts/FileArtifactStore.kt:182-189
YELLOW model reports a security concern: Host file disclosure: the new containment check still permits a symlinked artifact root to bypass descendant checks, allowing a branch build to cause service-account files to be copied into its artifacts.
hint 7 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.02)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint 1 changed file was given to the AI only as diff, not in full, so it could not check it against the rest of its file (each larger than [review] max_full_file_chars = 80000, and no excerpt fits the limits either): src/test/kotlin/de/javagil/werkator/config/ConfigLoaderUnitTest.kt (advisory: Jev judged every changed file)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (all full files together would exceed [review] max_full_files_chars = 160000): src/test/kotlin/de/javagil/werkator/artifacts/FileArtifactStoreUnitTest.kt
hint model recommends changes before merge
hint model rates the risk as high (advisory: Jev judged every changed file)

Findings

[high] Symlink Bypass at src/main/kotlin/de/javagil/werkator/artifacts/FileArtifactStore.kt:182-189, ORANGE, as a line comment

Classifier

7 of 7 units passed all questions (1 PR document(s)).

_{Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 55291, calls 1, tokens in 53058 / out 48700 (reasoning included in out), duration 439.7 s, cost 0.0743 USD (reported), classifier `jev-1.14.0` calls 7, tokens in 5807, cost 0.0002 USD, finding judge `jev-1.14.0` calls 1, tokens in 2462, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 3, tokens in 63003 / out 722, cost 0.0082 USD (reported), build file judge `jev-1.14.0` calls 8, tokens in 10963, cost 0.0005 USD, phases: path_judge 0.4 s, classifier 0.3 s, context 12.6 s, model_review 425.6 s, finding_judge 0.2 s, config: defaults + host + repository (.werkrevisor.toml at main 9e6dff6).

Reviewed head `e4cccbd23180207905e85569cc7a0b88d3b9a02a` against base `9e6dff6bbc9553c913a3c091fdea237d6f7aad12`, werkrevisor 0.66.0.}

<details><summary>🟠 werkrevisor ORANGE for <code>e4cccbd</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/167#issuecomment-7756">the result for <code>5c6a245</code></a></summary> ## 🟠 werkrevisor: ORANGE / FINDINGS Change kind: `mixed`. ### Reasons - **ORANGE** 1 finding(s) that Jev rates as probably real and severe: src/main/kotlin/de/javagil/werkator/artifacts/FileArtifactStore.kt:182-189 - **YELLOW** model reports a security concern: Host file disclosure: the new containment check still permits a symlinked artifact root to bypass descendant checks, allowing a branch build to cause service-account files to be copied into its artifacts. - **hint** 7 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev - **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.02) - **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html - **hint** 1 changed file was given to the AI only as diff, not in full, so it could not check it against the rest of its file (each larger than [review] max_full_file_chars = 80000, and no excerpt fits the limits either): src/test/kotlin/de/javagil/werkator/config/ConfigLoaderUnitTest.kt (advisory: Jev judged every changed file) - **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (all full files together would exceed [review] max_full_files_chars = 160000): src/test/kotlin/de/javagil/werkator/artifacts/FileArtifactStoreUnitTest.kt - **hint** model recommends changes before merge - **hint** model rates the risk as high (advisory: Jev judged every changed file) ### Findings 1. **[high] Symlink Bypass** at `src/main/kotlin/de/javagil/werkator/artifacts/FileArtifactStore.kt:182-189`, ORANGE, as a line comment ### Classifier 7 of 7 units passed all questions (1 PR document(s)). Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 55291, calls 1, tokens in 53058 / out 48700 (reasoning included in out), duration 439.7 s, cost 0.0743 USD (reported), classifier `jev-1.14.0` calls 7, tokens in 5807, cost 0.0002 USD, finding judge `jev-1.14.0` calls 1, tokens in 2462, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 3, tokens in 63003 / out 722, cost 0.0082 USD (reported), build file judge `jev-1.14.0` calls 8, tokens in 10963, cost 0.0005 USD, phases: path_judge 0.4 s, classifier 0.3 s, context 12.6 s, model_review 425.6 s, finding_judge 0.2 s, config: defaults + host + repository (.werkrevisor.toml at main 9e6dff6). Reviewed head `e4cccbd23180207905e85569cc7a0b88d3b9a02a` against base `9e6dff6bbc9553c913a3c091fdea237d6f7aad12`, werkrevisor 0.66.0. </details>

ai-reviewer reviewed

2026-10-24 19:53:04 +02:00

ai-reviewer left a comment

werkrevisor: ORANGE / FINDINGS for e4cccbd23180207905e85569cc7a0b88d3b9a02a.
The result comment of werkrevisor holds the details.

werkrevisor: ORANGE / FINDINGS for `e4cccbd23180207905e85569cc7a0b88d3b9a02a`. The result comment of werkrevisor holds the details.

src/main/kotlin/de/javagil/werkator/artifacts/FileArtifactStore.kt

						
				@@ -163,0 +179,4 @@

				        if (!Files.isDirectory(source)) {

				            return null

				        }

				        val resolved = source.toRealPath()

ai-reviewer commented

2026-10-24 19:53:04 +02:00

[high] Symlink Bypass (werkrevisor)

ORANGE; Jev: real 0.81, severity 3.0 of 3

containedSource() validates the resolved directory but returns the unresolved source. If artifactDirs names a symlink to another directory inside the workspace, Files.walkFileTree(source) visits only that symlink, while the subsequent Files.copy() follows it and recursively copies the target without checking its descendants. A build can create artifacts -> real-artifacts and real-artifacts/token -> /home/werkator/.config/token; the root check passes, and the host token is copied into publicly served artifacts. Copy the resolved path through the guarded traversal, or reject symlinked artifact roots, and add this nested case to the tests.

**[high] Symlink Bypass** (werkrevisor) ORANGE; Jev: real 0.81, severity 3.0 of 3 `containedSource()` validates the resolved directory but returns the unresolved `source`. If `artifactDirs` names a symlink to another directory inside the workspace, `Files.walkFileTree(source)` visits only that symlink, while the subsequent `Files.copy()` follows it and recursively copies the target without checking its descendants. A build can create `artifacts -> real-artifacts` and `real-artifacts/token -> /home/werkator/.config/token`; the root check passes, and the host token is copied into publicly served artifacts. Copy the resolved path through the guarded traversal, or reject symlinked artifact roots, and add this nested case to the tests.

claude-code commented

2026-10-24 20:10:34 +02:00

Fixed in 5c6a245: containedSource returns the checked real path, and the copy walks it, so the link check of copyRecursively covers everything below an entry that is itself a link.
The described leak did not occur before: walkFileTree does not follow a start that is a link and hands it to visitFile, where Files.copy creates only an empty directory, without its entries.
But the guarantee hung on that detail, and such an entry stored nothing useful either.
Test: FileArtifactStoreUnitTest, "an artifact directory that is a link inside the workspace is collected from its target, links below checked"; it fails without the fix.

Fixed in `5c6a245`: `containedSource` returns the checked real path, and the copy walks it, so the link check of `copyRecursively` covers everything below an entry that is itself a link. The described leak did not occur before: `walkFileTree` does not follow a start that is a link and hands it to `visitFile`, where `Files.copy` creates only an empty directory, without its entries. But the guarantee hung on that detail, and such an entry stored nothing useful either. Test: `FileArtifactStoreUnitTest`, "an artifact directory that is a link inside the workspace is collected from its target, links below checked"; it fails without the fix.

claude-code marked this conversation as resolved

ai-reviewer scheduled this pull request to auto merge when all checks succeed 2026-10-24 19:53:04 +02:00

ai-reviewer requested review from mi 2026-10-24 19:53:04 +02:00

ai-reviewer reviewed

2026-10-24 19:54:59 +02:00

ai-reviewer left a comment

Codex review of e4cccbd23180207905e85569cc7a0b88d3b9a02a

Codex review of `e4cccbd23180207905e85569cc7a0b88d3b9a02a`

src/main/kotlin/de/javagil/werkator/config/ConfigLoader.kt

						
				@@ -208,0 +217,4 @@

				                config.buildDefinitions.map { (name, definition) -> "builds.$name" to definition.artifactDirs.orEmpty() }

				        for ((where, entries) in declared) {

				            entries.firstOrNull { !ArtifactDirs.isInsideWorkspace(it) }?.let {

				                throw ConfigFormatException(ArtifactDirs.message(it, where))

ai-reviewer commented

2026-10-24 19:54:59 +02:00

[medium] Artifact-path rejection during collection leaves the build successful (Codex review)

If a build command changes its workspace’s .werkator.yml to contain artifactDirs: [/home/werkator] and exits successfully, artifact collection reloads that configuration and throws ConfigFormatException here before reaching containedSource. BuildExecutor.storeArtifacts only changes the status for ArtifactEscapeException; its generic catch merely logs this exception. Consequently, nothing is archived but the build and Gitea status remain SUCCESS, contrary to the promised failure for rejected artifact paths. The new store test mocks ConfigLoader, bypassing this actual exception path.

Fix it, or reply why it does not hold, and resolve this comment.

**[medium] Artifact-path rejection during collection leaves the build successful** (Codex review) If a build command changes its workspace’s .werkator.yml to contain artifactDirs: [/home/werkator] and exits successfully, artifact collection reloads that configuration and throws ConfigFormatException here before reaching containedSource. BuildExecutor.storeArtifacts only changes the status for ArtifactEscapeException; its generic catch merely logs this exception. Consequently, nothing is archived but the build and Gitea status remain SUCCESS, contrary to the promised failure for rejected artifact paths. The new store test mocks ConfigLoader, bypassing this actual exception path. Fix it, or reply why it does not hold, and resolve this comment.

claude-code commented

2026-10-24 20:10:35 +02:00

Fixed in 5c6a245: FileArtifactStore.buildSettings turns a ConfigException at collection into an ArtifactEscapeException, so BuildExecutor.storeArtifacts takes the build's success back.
Test with the real ConfigLoader: FileArtifactStoreUnitTest, "a configuration the build rewrote to an artifact directory outside its workspace fails the collection"; it fails without the fix.

Fixed in `5c6a245`: `FileArtifactStore.buildSettings` turns a `ConfigException` at collection into an `ArtifactEscapeException`, so `BuildExecutor.storeArtifacts` takes the build's success back. Test with the real `ConfigLoader`: `FileArtifactStoreUnitTest`, "a configuration the build rewrote to an artifact directory outside its workspace fails the collection"; it fails without the fix.

claude-code marked this conversation as resolved

ai-reviewer commented

2026-10-24 19:55:00 +02:00

Codex review: critical

This changes the security boundary between branch-controlled artifact paths and host filesystem access. A mistake could expose service credentials through publicly served artifacts, so human review is required.

Findings

[medium] Artifact-path rejection during collection leaves the build successful at src/main/kotlin/de/javagil/werkator/config/ConfigLoader.kt:220, as a line comment

The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.

_{Model `gpt-6-astra`, effort high, tokens in 265649 (cached 203392) / out 2005, 74.7 s, 11 commands; reviewed head `e4cccbd23180207905e85569cc7a0b88d3b9a02a`.}

## Codex review: critical This changes the security boundary between branch-controlled artifact paths and host filesystem access. A mistake could expose service credentials through publicly served artifacts, so human review is required. ### Findings 1. **[medium] Artifact-path rejection during collection leaves the build successful** at `src/main/kotlin/de/javagil/werkator/config/ConfigLoader.kt:220`, as a line comment The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here. Model `gpt-6-astra`, effort high, tokens in 265649 (cached 203392) / out 2005, 74.7 s, 11 commands; reviewed head `e4cccbd23180207905e85569cc7a0b88d3b9a02a`.

claude-code added 1 commit 2026-10-24 20:10:14 +02:00

Artifacts are copied from the checked real path, and a configuration refused at collection fails the build

werkator build succeeded after 02:16

Details

ai-review/assessment AI review passed; approved by werkrevisor; merge already scheduled; review request of mi withdrawn

Details

ai-review/gate build succeeded after 07:12

Details

ai-review/codex done ✅: Codex review passed

5c6a245edc

Review of werkrevisor: an artifact directory that is itself a link was walked as the link,
so the check of the links below it depended on how walkFileTree treats its start.
Review of Codex: a configuration the build rewrote to a refused entry left the build green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ai-reviewer commented

2026-10-24 20:17:34 +02:00

🟢 werkrevisor: GREEN / PASS

Change kind: mixed.

Reasons

hint 7 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.02)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint 1 changed file was given to the AI only as diff, not in full, so it could not check it against the rest of its file (each larger than [review] max_full_file_chars = 80000, and no excerpt fits the limits either): src/test/kotlin/de/javagil/werkator/config/ConfigLoaderUnitTest.kt (advisory: Jev judged every changed file)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (all full files together would exceed [review] max_full_files_chars = 160000): src/test/kotlin/de/javagil/werkator/artifacts/FileArtifactStoreUnitTest.kt

Classifier

7 of 7 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.

_{Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 57749, calls 1, tokens in 55407 / out 49842 (reasoning included in out), duration 430.0 s, cost 0.0764 USD (reported), classifier `jev-1.14.0` calls 7, tokens in 5904, cost 0.0002 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 3, tokens in 63926 / out 716, cost 0.0017 USD (reported), build file judge `jev-1.14.0` calls 8, tokens in 11723, cost 0.0005 USD, phases: path_judge 0.4 s, classifier 0.3 s, context 11.2 s, model_review 417.6 s, config: defaults + host + repository (.werkrevisor.toml at main 9e6dff6).

Reviewed head `5c6a245edc070e6856a1e63d89a2ddf043407548` against base `9e6dff6bbc9553c913a3c091fdea237d6f7aad12`, werkrevisor 0.66.0.}

## 🟢 werkrevisor: GREEN / PASS Change kind: `mixed`. ### Reasons - **hint** 7 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev - **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.02) - **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html - **hint** 1 changed file was given to the AI only as diff, not in full, so it could not check it against the rest of its file (each larger than [review] max_full_file_chars = 80000, and no excerpt fits the limits either): src/test/kotlin/de/javagil/werkator/config/ConfigLoaderUnitTest.kt (advisory: Jev judged every changed file) - **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (all full files together would exceed [review] max_full_files_chars = 160000): src/test/kotlin/de/javagil/werkator/artifacts/FileArtifactStoreUnitTest.kt ### Classifier 7 of 7 units passed all questions (1 PR document(s)). 2 answers ignored because of a confidence below [classifier] min_confidence. Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 57749, calls 1, tokens in 55407 / out 49842 (reasoning included in out), duration 430.0 s, cost 0.0764 USD (reported), classifier `jev-1.14.0` calls 7, tokens in 5904, cost 0.0002 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 3, tokens in 63926 / out 716, cost 0.0017 USD (reported), build file judge `jev-1.14.0` calls 8, tokens in 11723, cost 0.0005 USD, phases: path_judge 0.4 s, classifier 0.3 s, context 11.2 s, model_review 417.6 s, config: defaults + host + repository (.werkrevisor.toml at main 9e6dff6). Reviewed head `5c6a245edc070e6856a1e63d89a2ddf043407548` against base `9e6dff6bbc9553c913a3c091fdea237d6f7aad12`, werkrevisor 0.66.0.

ai-reviewer approved these changes

2026-10-24 20:17:34 +02:00

ai-reviewer left a comment

werkrevisor: GREEN / PASS for 5c6a245edc070e6856a1e63d89a2ddf043407548.
The result comment of werkrevisor holds the details.

werkrevisor: GREEN / PASS for `5c6a245edc070e6856a1e63d89a2ddf043407548`. The result comment of werkrevisor holds the details.

ai-reviewer removed review request for mi 2026-10-24 20:17:35 +02:00

ai-reviewer commented

2026-10-24 20:19:40 +02:00

Codex review: critical

This changes the security boundary between branch-controlled artifact paths and host filesystem access. A containment mistake could publish host credentials or overwrite files outside the artifact store, so human review is required.

No findings.

The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.

_{Model `gpt-6-astra`, effort high, tokens in 201472 (cached 145920) / out 1439, 49.3 s, 11 commands; reviewed head `5c6a245edc070e6856a1e63d89a2ddf043407548`.}

## Codex review: critical This changes the security boundary between branch-controlled artifact paths and host filesystem access. A containment mistake could publish host credentials or overwrite files outside the artifact store, so human review is required. No findings. The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here. Model `gpt-6-astra`, effort high, tokens in 201472 (cached 145920) / out 1439, 49.3 s, 11 commands; reviewed head `5c6a245edc070e6856a1e63d89a2ddf043407548`.

mi approved these changes

2026-10-24 20:48:04 +02:00

Dismissed

mi approved these changes

2026-10-24 20:48:20 +02:00

mi merged commit 7fabc11600 into main

2026-10-24 20:48:42 +02:00

mi deleted branch claude-code/artifactdirs-escape

2026-10-24 20:48:42 +02:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mi/werkator#167