[bug-fix] Harden persisted workflow run artifact access against symlinks by github-actions[bot] · Pull Request #4930 · github/spec-kit

github-actions · 2026-10-12T14:28:13Z

Bug fix — persisted-run-symlinks

Proposed fix for issue #4914, applying the remediation from the bug assessment.

Verdict: valid · Severity: medium

Summary

Introduces a shared persisted-run storage boundary that rejects symlinked run directories and artifact files before workflow state, input, snapshot, or log access. Resume, status, run listing, and snapshot creation now fail closed consistently, with the policy documented for workflow users and maintainers.

Changes

File	Change	Notes
`src/specify_cli/workflows/engine.py`	modified	Added shared run-directory/artifact validation and no-follow artifact opens; routed state load/save, input fallback, log writes, workflow snapshot creation, resume snapshot reads, and run listing through it.
`src/specify_cli/workflows/command_status.py`	modified	Converts unsafe storage failures from run listing into clean CLI errors.
`tests/specify_cli/workflows/test_run_state_storage.py`	added test	Covers symlinked run directories and state, input, workflow snapshot, and log artifacts for reads and writes.
`workflows/README.md`	modified	Documents the persisted-run symlink policy.
`workflows/ARCHITECTURE.md`	modified	Documents the storage boundary and no-follow behavior.

Tests Added or Updated

tests/specify_cli/workflows/test_run_state_storage.py::test_load_rejects_symlinked_run_directory — rejects redirected per-run directories.
tests/specify_cli/workflows/test_run_state_storage.py::test_run_state_rejects_symlinked_persisted_artifacts — rejects redirected state and input artifacts.
tests/specify_cli/workflows/test_run_state_storage.py::test_run_state_rejects_symlinked_artifact_writes — rejects redirected state, input, and log writes.
tests/specify_cli/workflows/test_run_state_storage.py::test_workflow_snapshot_artifact_is_checked — rejects redirected workflow snapshots.

Local Verification

Commands run: python3 -m compileall -q src/specify_cli/workflows/engine.py src/specify_cli/workflows/command_status.py tests/specify_cli/workflows/test_run_state_storage.py → passed.
Commands run: git diff --check → passed.
Targeted pytest command → not run because pytest is unavailable in the environment; no dependencies were installed.

Deviations from Assessment

The current branch has no workflow definition command, so no command-specific change was possible there; persisted snapshot access is covered in the shared engine layer.
The implementation uses O_NOFOLLOW where available plus immediate symlink checks; no concurrent mutation test was added because the targeted test runner is unavailable.

Risks & Review Notes

Existing workflows that intentionally relocate run storage through symlinks will now be rejected as an intentional compatibility trade-off.
Filesystem behavior around concurrent replacement remains platform-dependent; no-follow leaf opens reduce this risk where supported, while the explicit policy rejects static symlinked paths.

Refs #4914 · cc @markuswondrak

Fixes [Bug]: Harden persisted workflow run artifact access against symlinks #4914

Generated by 🛠️ Fix Bug from Labeled Issue for #4914 · copilot · gpt52codex · 6.54 AIC · ⌖ 0.836 AIC · ⊞ 24.4K · ◷

Apply the remediation from the bug assessment on issue #4914. Refs #4914 Assisted-by: GitHub Copilot (model: gpt-5.2-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Snapshot reads retain a symlink race, Ruff fails on an unused variable, and key consumer regressions remain untested.

4 open findings

Remove unused run directory assignment · New
Prevent symlink race when reopening workflow snapshots · New
Test safe list-runs behavior with symlinked storage · New
Add resume and execute symlink regression tests · New

What changed in this PR

Adds a shared security boundary for persisted workflow-run artifacts to reject symlink redirection.

Changes:

Validates run directories and artifacts before access.
Adds no-follow file opening and CLI error handling.
Documents and tests the symlink policy.

File	Description
`src/specify_cli/workflows/engine.py`	Implements storage validation and protected artifact access.
`src/specify_cli/workflows/command_status.py`	Handles unsafe listing failures.
`tests/specify_cli/workflows/test_run_state_storage.py`	Adds symlink rejection tests.
`workflows/README.md`	Documents user-facing policy.
`workflows/ARCHITECTURE.md`	Documents the storage boundary.

🧠 Review effort: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot · 2026-10-12T19:37:36Z

+        err = cli._error_console(json_output)
+        try:
+            runs = engine.list_runs()
+        except (OSError, ValueError) as exc:
+            err.print(f"[red]Error:[/red] {cli._escape_markup(str(exc))}")


Added workflow status --json regressions for symlinked run directories and state.json, asserting clean failure and stderr-only output. Fixed in 79f1da7.

The workflow status --json listing regressions for symlinked run directories and state.json are present and pass on the current branch (02159bf0).

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

Copilot

🟡 Changes recommended

A parent-directory replacement race remains, and an existing unreadable-state regression test is broken.

3 open findings

3 resolved since last review

🧠 Review effort: Balanced

Copilot · 2026-10-12T21:05:33Z

            if state_path.exists():
                try:
-                    with open(state_path, encoding="utf-8") as f:
+                    with self._open_artifact(state_path, "r", encoding="utf-8") as f:


The unreadable-state regression now patches RunState._open_artifact and still asserts that the bad row is skipped; it passes on the current branch (02159bf0).

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

Copilot

🟡 Changes recommended

The unconditional Unix-specific filesystem requirements break persisted workflow operations on supported Windows systems.

3 open findings

1 resolved since last review

Prevent run-directory symlink races during artifact access

🧠 Review effort: Balanced

Copilot · 2026-10-12T21:05:33Z

+            directory_flag is None
+            or nofollow is None
+            or os.open not in os.supports_dir_fd
+        ):
+            raise OSError("Secure workflow run-directory access is unavailable")


Added a checked path-based fallback for platforms without directory-descriptor/no-follow support, including execute/resume/listing and symlink regressions. Committed as 02159bf0; the architecture note documents that this fallback cannot prevent concurrent path replacement.

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

+            RunState._open_artifact(
+                artifact, "r", dir_fd=run_fd, encoding="utf-8"
+            )


+                try:
+                    RunState._validate_fallback_path(tmp_path)
+                    tmp_path.unlink()
+                except (OSError, ValueError):


Fix persisted-run-symlinks: reject symlinked workflow artifacts

1650af6

Apply the remediation from the bug assessment on issue #4914. Refs #4914 Assisted-by: GitHub Copilot (model: gpt-5.2-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added automated bug-fix Trigger the bug-fix agentic workflow labels Oct 12, 2026

github-actions Bot mentioned this pull request Oct 12, 2026

[Bug]: Harden persisted workflow run artifact access against symlinks #4914

Open

3 tasks

github-code-quality Bot found potential problems Oct 12, 2026

View reviewed changes

Comment thread src/specify_cli/workflows/engine.py Fixed

mnriem requested a balanced review from Copilot October 12, 2026 14:44

Copilot started reviewing on behalf of mnriem October 12, 2026 14:45 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

mnriem marked this pull request as ready for review October 12, 2026 19:32

mnriem self-requested a review as a code owner October 12, 2026 19:32

Copilot started work on behalf of mnriem October 12, 2026 19:32 View session

Fix persisted workflow snapshot symlinks

79f1da7

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

Copilot finished work on behalf of mnriem October 12, 2026 19:38

mnriem requested a balanced review from Copilot October 12, 2026 20:05

Copilot started reviewing on behalf of mnriem October 12, 2026 20:06 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Copilot started work on behalf of mnriem October 12, 2026 20:14 View session

Secure workflow artifacts against run-directory swaps

bd3507f

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

Copilot finished work on behalf of mnriem October 12, 2026 20:23

mnriem requested a balanced review from Copilot October 12, 2026 20:48

Copilot started reviewing on behalf of mnriem October 12, 2026 20:49 View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Copilot started work on behalf of mnriem October 12, 2026 20:57 View session

Support workflow persistence without dir-fd APIs

02159bf

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

github-code-quality Bot found potential problems Oct 12, 2026

View reviewed changes

Copilot finished work on behalf of mnriem October 12, 2026 21:06

mnriem added the bug-test label Oct 12, 2026

Navigation Menu

Repository navigation

[bug-fix] Harden persisted workflow run artifact access against symlinks - #4930

[bug-fix] Harden persisted workflow run artifact access against symlinks#4930
github-actions[bot] wants to merge 4 commits into
mainfrom
fix/4914-persisted-run-symlinks-2a3452971b7ee167

github-actions Bot commented Oct 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 12, 2026

Uh oh!

Copilot AI Oct 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Oct 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 12, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

github-actions Bot commented Oct 12, 2026

Bug fix — persisted-run-symlinks

Summary

Changes

Tests Added or Updated

Local Verification

Deviations from Assessment

Risks & Review Notes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants