Skip to content

test: make the verifier's fake curl robust to whitespace in sys.executable - #5362

Open
mehdikarimi55 wants to merge 2 commits into
github:mainfrom
mehdikarimi55:fix/5360-verifier-fixture-whitespace-shebang
Open

mehdikarimi55 wants to merge 2 commits into
github:mainfrom
mehdikarimi55:fix/5360-verifier-fixture-whitespace-shebang

Conversation

@mehdikarimi55

Copy link
Copy Markdown

Fixes #5360

Problem

tests/test_workflow_step_submission.py::test_fetch_keeps_malicious_text_in_one_argument_even_if_validation_is_bypassed fails 4 times (one per parametrized suffix) when the spec-kit checkout lives under a path containing whitespace — e.g. ~/Downloads/GitHub Contribution/spec-kit:

E   step_submission_verifier.Blocked: cannot download https://...: [Errno 2] No such file or directory: 'curl'

Root cause

The fixture writes its fake curl with a #!{sys.executable} shebang. On macOS the XNU kernel splits the shebang line on whitespace, so an interpreter path like /Users/me/Downloads/GitHub Contribution/spec-kit/.venv/bin/python is parsed as interpreter /Users/me/Downloads/GitHub with argument Contribution/.... execve fails with ENOENT and the test reports "No such file or directory: 'curl'". GitHub CI never sees this (runner paths have no spaces), but desktop checkouts with a space in the path hit it deterministically.

Fix

Route the fake curl through #!/bin/sh with a shlex.quoted interpreter — the same established pattern as _ensure_test_python_on_path() in tests/extensions/test_extension_agent_context.py and make_python3_path_shim() in tests/parity_helpers.py. With -c, sys.argv[1:] is exactly the argument list curl was invoked with, so the test's assertions (len(args) == 14, args[-1] == url) are unchanged.

Test-only change; no product code touched.

Evidence

Reproduction of the kernel behavior outside the suite:

mkdir -p "/tmp/space test"
ln -s "$(command -v python3)" "/tmp/space test/python"
printf '#!/tmp/space test/python\nprint(1)\n' > /tmp/script
chmod +x /tmp/script
/tmp/script   # macOS: OSError: [Errno 2] No such file or directory

And in the suite, in the affected environment (macOS 15 arm64, checkout under a path with a space):

.venv/bin/python -m pytest tests/test_workflow_step_submission.py -q
# before: 4 failed   after: 271 passed

AI disclosure

Agent-authored commit and PR — OpenCode running GLM 5.3 Flash (Z.ai), agent mode under human supervision by @mehdikarimi55. The agent performed reproduction, root-cause analysis, the code change, and test execution; changes were reviewed by the account owner before submission.

…table

On macOS the kernel splits a shebang line on whitespace, so the fixture's
"#!{sys.executable}" shebang fails with ENOENT whenever the checkout
lives under a path containing a space (e.g. "GitHub Contribution/"),
producing 4 test failures. Route the fake curl through /bin/sh with a
shlex.quote'd interpreter, matching the existing pattern in
tests/extensions/test_extension_agent_context.py; with -c, sys.argv[1:]
stays exactly the curl argument list, so the assertions are unchanged.

Fixes github#5360

AI disclosure: agent-authored commit — OpenCode running GLM 5.3 Flash
(Z.ai), agent mode under human supervision by mehdikarimi55; root-cause
analysis, code change, and test execution performed by the agent.
Copilot AI auto review requested due to automatic review settings November 10, 2026 10:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported, and the fixture change preserves the existing security assertions while addressing interpreter paths containing whitespace.

No open issues 🎉

What changed in this PR

This test-only PR hardens the workflow-step submission verifier’s fake curl fixture so it works when sys.executable is located under a path containing whitespace, particularly on macOS.

Changes:

  • Replaces the whitespace-sensitive Python shebang with a /bin/sh wrapper.
  • Uses shlex.quote to safely invoke Python while preserving the original curl argument list.
  • Retains the existing malicious-input and injection-prevention assertions.
File Description
tests/​test_workflow_step_submission.py Updates the fake curl fixture to safely invoke Python from interpreter paths containing whitespace.

Review effort: Lite (auto)

@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Nov 10, 2026
@mnriem
mnriem requested an auto review from Copilot November 10, 2026 14:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The regression test does not reliably verify the reported shebang fix because it uses an ambient interpreter path without whitespace, allowing the old behavior to pass on CI.

1 open issue

Review effort: Lite (auto)

Comment thread tests/test_workflow_step_submission.py Resolved
@mnriem

mnriem commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

@mnriem mnriem added the author-awaiting Waiting on author response label Nov 10, 2026
Copilot review noted the regression test still ran with the ambient
sys.executable, whose path has no whitespace on CI, so restoring the
old direct shebang would not fail there. Reach the interpreter through
a symlink inside a directory whose name contains a space, making the
direct-shebang fixture fail with ENOENT on every machine and locking
in the /bin/sh fix.

Assisted-by: GLM (model: GLM 5.3 Flash, supervised by mehdikarimi55)

AI disclosure: agent-authored commit - GLM 5.3 Flash, supervised by
mehdikarimi55; test change and red/green verification performed by
the agent.
Copilot AI auto review requested due to automatic review settings November 11, 2026 04:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported, and the fixture change addresses interpreter paths containing whitespace while preserving argument-integrity coverage.

No open issues 🎉

1 resolved since last review

Review effort: Lite (auto)

This branch has not been deployed

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

Labels

author-awaiting Waiting on author response triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workflow-step verifier test fixture breaks when the checkout path contains whitespace (4 failures on macOS)

3 participants