Skip to content

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

Open

Description

@markuswondrak

Bug Description

Workflow run storage guards validate shared ancestors through .specify/workflows/runs, but RunState.load() and persisted snapshot readers still follow a symlinked per-run directory or artifact file. This is a cross-cutting workflow-run storage gap that predates workflow definition: workflow resume already reads runs/<run_id>/workflow.yml, and run-state loading already reads state.json beneath the same directory.

Steps to Reproduce

  1. Initialize a Specify project and run a workflow that pauses, creating .specify/workflows/runs/<run_id>/state.json and workflow.yml.
  2. Move that run directory to an external location, then replace runs/<run_id> with a symbolic link to that external directory. Alternatively, replace its state.json or workflow.yml with a symbolic link to an external valid artifact.
  3. Run specify workflow resume <run_id>; on this PR, run specify workflow definition <run_id> --json as well.

Expected Behavior

All workflow-run consumers reject persisted run directories and artifact files that violate one shared, documented symlink policy before reading or writing them. The failure should be consistent across RunState.load, resume, status, and definition.

Actual Behavior

The current shared-ancestor guard accepts the symlinked per-run path. The state and snapshot readers can follow it, so resume reads externally redirected artifacts. workflow definition additionally emits the externally redirected snapshot.

Specify CLI Version

1.1.3.dev0 (v0.1.10-1615-g57b41362 working tree)

AI Agent

Not applicable to the affected workflow behavior.

Operating System

Linux 6.17.0-1032-oem x86_64 GNU/Linux

Python Version

Python 3.14.4

Proposed Scope

Establish one persisted-run storage boundary rather than adding command-local checks. Cover all readers and writers of run artifacts, including RunState.load, persisted definition reads, resume, status, and workflow definition.

Decide and document whether the policy rejects all symlinks below runs/<run_id> or permits only links confined to the run root. Make checks and opens resistant to replacement races if the adopted threat model requires it.

Acceptance Criteria

  • Per-run directories and persisted artifacts follow one documented symlink policy.
  • All run-state consumers receive consistent failures.
  • Negative tests cover symlinked run directories and persisted artifact files across shared behavior.

Additional Context

Reported by Copilot review on PR #4912: #4912 (comment)

AI Disclosure

Filed and updated by Markus Wondrak with OpenCode (github-copilot/gpt-5.6-terra), default reasoning, autonomous mode. The agent traced the existing run-storage paths, drafted the reproduction and issue, and updated it to match the repository bug-report template; no claim of human line-by-line review is made.

Activity

  1. changed the title [-]Harden persisted workflow run artifact access against symlinks[/-] [+][Bug]: Harden persisted workflow run artifact access against symlinks[/+] on Oct 10, 2026
  2. github-actions commented on Oct 12, 2026

    @github-actions
    Contributor

    Bug assessment — persisted-run-symlinks: Valid · severity medium


    Bug Assessment: Harden persisted workflow run artifact access against symlinks

    Report (summarized)

    The report states that workflow-run storage guards validate shared ancestors through .specify/workflows/runs, but consumers still follow symlinked per-run directories or artifact files. It identifies RunState.load() and persisted workflow snapshots used by resume (and the reported workflow definition command on PR #4912) as affected, and requests one documented policy covering readers and writers. The report includes a reproduction involving a symlinked run directory or state.json/workflow.yml and expects consistent rejection across resume, status, and definition.

    Additional context: the report links to the Copilot review discussion on github/spec-kit#4912. No external content was required to assess the issue.

    Symptom

    The current workflow storage guard rejects symlinked shared directories such as .specify/workflows/runs, but it does not reject a symlink below that directory. As a result, persisted state and snapshot reads can follow a run-directory or artifact symlink to an external location, while writes can also target a redirected path; commands do not therefore enforce one consistent persisted-run boundary.

    Reproduction

    1. Initialize a Specify project and create a paused workflow run so .specify/workflows/runs/<run_id>/state.json and workflow.yml exist.
    2. Replace the per-run directory with a symlink to a directory outside the project, or replace state.json or workflow.yml with a symlink to an external file.
    3. Run specify workflow resume <run_id> or specify workflow status <run_id>.
    4. If using the workflow definition <run_id> --json command from PR feat: add specify workflow definition for run-scoped definitions #4912, invoke it as well.

    The exact command and option set for producing the paused run is not specified in the report: [NEEDS CLARIFICATION: confirm the minimal non-interactive workflow and the exact workflow definition command available in the target release].

    Suspected Code Paths

    • src/specify_cli/workflows/_commands.py:166-177 — _reject_unsafe_workflow_storage() checks .specify, .specify/workflows, and .specify/workflows/runs, but stops at the shared runs directory and does not inspect <run_id> or its files.
    • src/specify_cli/workflows/engine.py:734-778 — RunState.runs_dir, save(), and append_log() construct paths below runs/<run_id> and use mkdir, open, and os.replace without rejecting a symlinked run directory or artifact leaf.
    • src/specify_cli/workflows/engine.py:812-834 — RunState.load() opens runs/<run_id>/state.json directly. The validated run ID prevents lexical traversal, but the filesystem path can still resolve through a symlinked directory or file.
    • src/specify_cli/workflows/engine.py:923-925 — legacy input loading checks inputs.json and then opens it without a symlink-policy check.
    • src/specify_cli/workflows/engine.py:1185-1190 — WorkflowEngine.resume() checks run_copy.exists() and then loads runs/<run_id>/workflow.yml; both operations follow a symlinked run directory or snapshot file.
    • src/specify_cli/workflows/engine.py:1484-1500 — WorkflowEngine.list_runs() iterates run entries and opens each state.json without rejecting symlinked per-run directories or state files, affecting workflow status when listing runs.
    • src/specify_cli/workflows/command_resume.py:48-74 — the CLI preloads persisted state through RunState.load(), so the shared gap is reached before the later ownership checks.
    • src/specify_cli/workflows/command_status.py:31-48 — single-run status also delegates directly to RunState.load() and therefore inherits the same behavior.

    Root Cause Hypothesis

    Confidence: high. The existing protection is an ancestor-only guard: it verifies that the shared storage directories are not symlinks before workflow commands run, but it treats all descendants as trusted. The persistence layer subsequently uses ordinary path-based opens and atomic replacement, which follow descendant symlinks. Run ID validation closes path traversal through user input but does not establish an object-level boundary for the run directory or artifact files. The same pattern appears in both read paths (state.json, inputs.json, workflow.yml) and write paths (state.json, inputs.json, log.jsonl, and the initial snapshot).

    Proposed Remediation

    Preferred: introduce a shared persisted-run storage boundary helper in the workflow persistence layer. It should validate the lexical run ID, reject any symlink component below .specify/workflows/runs, require the per-run directory to be a real directory, and reject symlinked artifact leaves before every read or write. Route RunState.load(), save(), append_log(), WorkflowEngine.execute() snapshot creation, and list_runs() through that helper so resume, status, and future persisted-artifact consumers receive the same failure behavior. Document the policy in the workflow architecture/README: the simplest defensible rule is to reject all symlinks below runs/, rather than permitting links whose targets happen to remain inside the project.

    For race resistance, use descriptor-relative operations with O_NOFOLLOW/dir_fd where supported, or otherwise verify the relevant directory and leaf immediately before opening/replacing and fail closed on replacement races. Preserve the existing user-facing error convention by translating unsafe storage into a clear CLI error rather than silently skipping or following the path.

    Alternatives:

    • Resolve each candidate path and require it to remain under the canonical runs root. This is simpler but permits symlinks within the root and is vulnerable to replacement races unless paired with no-follow file-descriptor operations.
    • Copy/read snapshots through a trusted staging mechanism and treat the run directory as immutable after creation. This improves integrity but is a larger behavioral change and does not by itself protect legacy state.json/inputs.json reads.

    Files likely to change:

    • src/specify_cli/workflows/engine.py
    • src/specify_cli/workflows/_commands.py (shared command-level error integration, if needed)
    • src/specify_cli/workflows/command_resume.py
    • src/specify_cli/workflows/command_status.py (only if command-specific handling is required)
    • workflows/ARCHITECTURE.md and/or workflows/README.md for the persisted-run symlink policy
    • tests/specify_cli/workflows/test_command_resume.py
    • tests/specify_cli/workflows/test_command_status.py
    • a focused engine/run-state test module if one is added for the shared helper

    Tests to add or update:

    • Reject a symlinked <run_id> directory during RunState.load().
    • Reject a symlinked state.json, inputs.json, workflow.yml, and log.jsonl for both reads and writes.
    • Verify workflow resume, single-run workflow status, and run listing fail consistently and do not read the external target.
    • Verify normal non-symlinked runs retain existing resume/status behavior.
    • If the adopted threat model includes concurrent filesystem mutation, add a targeted no-follow/race-safe test for the chosen descriptor-based implementation, with platform guards where required.

    Risks & Considerations

    • Rejecting existing symlinked run artifacts may affect users who intentionally relocate workflow storage; this is an intentional compatibility trade-off and should be documented.
    • os.replace and append mode have different symlink behavior, so the implementation must cover each write path rather than only reads.
    • Cross-platform behavior differs for O_NOFOLLOW and directory descriptors; retain a safe fallback that rejects symlinks even where race-resistant primitives are unavailable.
    • A local actor generally needs write access to the project workspace to plant these symlinks, so this is not equivalent to a remote unauthenticated file-read vulnerability. It can still cause unintended reads, writes, or workflow-state corruption when the CLI is run against a manipulated workspace.
    • Existing broad exception handling in resume should preserve the repository's current user-facing error shape while ensuring unsafe-path failures are not converted into success-shaped results.

    Open Questions

    • [NEEDS CLARIFICATION: Should the policy reject every symlink below runs/, including links whose targets remain within the project, or allow a constrained in-root form?]
    • [NEEDS CLARIFICATION: Is workflow definition <run_id> --json part of the target branch/release, or only the PR context referenced by the report?]
    • [NEEDS CLARIFICATION: Is resistance to concurrent symlink replacement within the supported threat model, or is a static symlink check sufficient for this release?]

    Generated by 🐛 Assess Bug from Labeled Issue for #4914 · copilot · gpt52codex · 2.82 AIC · ⌖ 0.667 AIC · ⊞ 22.1K · ◷

  3. added
    bug-fixTrigger the bug-fix agentic workflow
    on Oct 12, 2026
  4. github-actions commented on Oct 12, 2026

    @github-actions
    Contributor

    Pull request created: #4930

    Generated by Fix Bug from Labeled Issue · copilot · gpt52codex · 6.54 AIC · ⌖ 0.836 AIC · ⊞ 24.4K

  5. github-actions commented on Oct 12, 2026

    @github-actions
    Contributor

    Draft PR created for persisted-run-symlinks: review the proposed changes. The fix adds a shared persisted-run storage boundary that rejects symlinked run directories and state, input, snapshot, and log artifacts. Please review the draft and validate it in the configured stage-3 bug-test workflow.

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

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions