-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ci): reap fatal OpenCode process groups #813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d0a36e9
fix(ci): reap fatal OpenCode process groups
seonghobae d477b18
test(docs): require authoritative setsid citation metadata
seonghobae b968b39
docs: correct setsid primary-source citation
seonghobae 6748ef1
fix(security): fail dependency-review support closed
seonghobae 63d3478
test: capture dependency review curl transport failure
seonghobae 8f898bf
fix(ci): fail closed on dependency review transport errors
seonghobae c244b2b
docs: define fail-closed dependency review probe contract
seonghobae 1779da4
docs: record dependency review transport failure closure
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Dependency-review support probe fail-closed contract | ||
|
|
||
| ## Incident | ||
|
|
||
| The required central `Security Scan` workflow probes GitHub's dependency-review compare endpoint before invoking `actions/dependency-review-action`. The probe already treated every HTTP status other than 200 as unavailable evidence. However, the shell command substitution appended `|| true`, so a transport-level `curl` failure could be converted into shell success. Because `curl --write-out '%{http_code}'` can emit an HTTP status even when the transfer itself later fails, an output of `200` paired with a nonzero curl exit status could incorrectly set `supported=true`. | ||
|
|
||
| This is an evidence-integrity defect rather than a dependency vulnerability. A required security gate must not claim the dependency-review prerequisite is available unless both the transport command and the API status prove success. | ||
|
|
||
| ## Decision | ||
|
|
||
| The support probe now has two independent fail-closed conditions: | ||
|
|
||
| 1. `curl` must exit successfully under the existing ten-second connection timeout and thirty-second total timeout; and | ||
| 2. the returned status text must be exactly `200` for the exact pull-request base/head comparison. | ||
|
|
||
| The workflow discards the untrusted response body and writes `supported=true` only after both conditions pass. Timeout, partial transfer, connection failure, TLS failure, malformed or empty status output, HTTP 403/404, and every other non-200 response terminate the job. The dependency-review action, its immutable pin, its severity threshold, workflow permissions, API endpoint, API-version header, and credential identity are unchanged. | ||
|
|
||
| ## Test-first evidence | ||
|
|
||
| `tests/test_dependency_review_support_probe.py` executes the exact shell body extracted from `.github/workflows/security-scan.yml` with an injected fake `curl`. The regression makes `curl` print HTTP `200` and then exit with code 18, representing a partial-transfer failure. The accepted contract is that the shell step exits nonzero, emits the existing fail-closed diagnostic, and never writes `supported=true` to `GITHUB_OUTPUT`. | ||
|
|
||
| The regression was committed before the workflow repair so the defect remained observable independently of the implementation change. | ||
|
|
||
| ## Operational interpretation | ||
|
|
||
| A failed support probe means dependency-review assurance is unavailable for that exact base/head pair. It is not permission to skip the dependency-review job and it must not be reclassified as success because another scanner passed. Retry after an infrastructure or GitHub service failure; remediate repository feature or authorization configuration for persistent 403/404 responses. Only a successful probe followed by the required dependency-review action can satisfy this part of the central supply-chain gate. | ||
|
|
||
| ## Rollback | ||
|
|
||
| Rollback requires an independently reviewed replacement that preserves both transport-success and exact-HTTP-200 evidence. Restoring `|| true`, treating a nonzero curl exit as advisory, or allowing 403/404 to produce a successful skip would reintroduce the fail-open condition and is not an acceptable rollback. | ||
|
|
||
| ## APA 7th references | ||
|
|
||
| GitHub. (2026). *REST API endpoints for dependency review*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/rest/dependency-graph/dependency-review | ||
|
|
||
| The curl project. (2026). *curl: How to use*. Retrieved August 7, 2026, from https://curl.se/docs/manpage.html | ||
|
|
||
| The curl project. (2026). *libcurl error codes*. Retrieved August 7, 2026, from https://curl.se/libcurl/c/libcurl-errors.html |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # OpenCode fatal-provider process-group termination | ||
|
|
||
| ## Incident | ||
|
|
||
| The exact-head coverage-evidence job for `.github` pull request #799 reached the repository test suite but did not complete inside its bounded measurement step. A focused reproduction identified `test_fatal_provider_error_kills_hung_opencode_run_early`: the launcher detected a fatal provider event and terminated the `timeout` wrapper, while a descendant fake `opencode` process could remain alive with inherited output pipes. The parent Python process then waited for end-of-file even though the launcher had returned. | ||
|
|
||
| ## Decision | ||
|
|
||
| Each bounded `opencode run` starts in a new session with `setsid`. On a structured fatal-provider event, the launcher sends `SIGTERM` to the negative process-group identifier, waits for bounded group disappearance, and then sends `SIGKILL` to the same group if necessary. The ordinary timeout contract remains `timeout --kill-after=30s`; only the early-fatal cleanup boundary changes. | ||
|
|
||
| The group signal is deliberately scoped to the session created for one model attempt. It does not target the workflow shell, unrelated model attempts, or the runner process. The production Ubuntu image already installs `util-linux`, which supplies `setsid`. | ||
|
|
||
| ## Verification | ||
|
|
||
| The existing behavioral regression uses a fake provider that emits a fatal structured event and sleeps for 120 seconds. Before the change, the test exceeded its 30-second subprocess boundary because a descendant retained the capture pipes. With process-group termination, it completes in under 25 seconds and the complete model-pool test file remains eligible for the exact-head coverage job. Shell syntax validation and the repository-wide evidence command remain required before merge. | ||
|
|
||
| ## Rollback | ||
|
|
||
| Rollback requires an independently reviewed change and a replacement mechanism that proves every descendant of a fatal model attempt is reaped without terminating unrelated runner work. Restoring PID-only termination is not acceptable because it reintroduces the pipe-retention failure mode. | ||
|
|
||
| ## APA 7th references | ||
|
|
||
| IEEE & The Open Group. (2024). *The Open Group base specifications issue 8: System interfaces, `kill()`*. https://pubs.opengroup.org/onlinepubs/9799919799/functions/kill.html | ||
|
|
||
| Free Software Foundation. (n.d.). *GNU Coreutils manual: `timeout`: Run a command with a time limit*. Retrieved August 7, 2026, from https://www.gnu.org/software/coreutils/manual/html_node/timeout-invocation.html | ||
|
|
||
| Linux man-pages project. (2026, February 8). *setsid(2) — Linux manual page* (Linux man-pages 6.18). https://man7.org/linux/man-pages/man2/setsid.2.html |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| """Behavioral regressions for the dependency-review capability probe.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import stat | ||
| import subprocess | ||
| import textwrap | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| REPO_ROOT = Path(__file__).resolve().parents[1] | ||
|
|
||
|
|
||
| def _support_probe_script() -> str: | ||
| """Extract the exact shell body used by the dependency-review support step.""" | ||
| workflow = ( | ||
| REPO_ROOT / ".github" / "workflows" / "security-scan.yml" | ||
| ).read_text(encoding="utf-8") | ||
| step_marker = " - name: Check dependency review support\n" | ||
| step_start = workflow.index(step_marker) | ||
| run_marker = " run: |\n" | ||
| run_start = workflow.index(run_marker, step_start) + len(run_marker) | ||
| run_end = workflow.index("\n - name:", run_start) | ||
| return textwrap.dedent(workflow[run_start:run_end]) | ||
|
|
||
|
|
||
| def test_dependency_review_probe_rejects_curl_failure_with_http_200(tmp_path) -> None: | ||
| """A transport failure must fail closed even when curl printed HTTP 200.""" | ||
| fake_bin = tmp_path / "bin" | ||
| fake_bin.mkdir() | ||
| fake_curl = fake_bin / "curl" | ||
| fake_curl.write_text("#!/bin/sh\nprintf '200'\nexit 18\n", encoding="utf-8") | ||
| fake_curl.chmod(fake_curl.stat().st_mode | stat.S_IXUSR) | ||
| github_output = tmp_path / "github-output.txt" | ||
| environment = os.environ.copy() | ||
| environment.update( | ||
| { | ||
| "PATH": f"{fake_bin}:{environment['PATH']}", | ||
| "GH_TOKEN": "test-token", | ||
| "BASE_SHA": "a" * 40, | ||
| "HEAD_SHA": "b" * 40, | ||
| "REPOSITORY": "ContextualWisdomLab/example", | ||
| "GITHUB_API_URL": "https://api.github.invalid", | ||
| "GITHUB_OUTPUT": str(github_output), | ||
| } | ||
| ) | ||
|
|
||
| completed = subprocess.run( | ||
| ["bash", "-c", _support_probe_script()], | ||
| cwd=REPO_ROOT, | ||
| env=environment, | ||
| text=True, | ||
| capture_output=True, | ||
| check=False, | ||
| ) | ||
|
|
||
| assert completed.returncode != 0 | ||
| assert "Failing closed" in f"{completed.stdout}\n{completed.stderr}" | ||
| assert not github_output.exists() or "supported=true" not in github_output.read_text( | ||
| encoding="utf-8" | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.