Skip to content

Commit 76621d9

Browse files
authored
CI Workflow updates (#313)
* Harden GitHub workflows against unsafe writes * Harden agent verification workflows * Extract archives atomically in verification * Harden workflow agent execution and URL checks * Harden workflow sandbox process handling * Retry transient DNS failures during validation * Harden protocol matrix artifact handling
1 parent e6b8eec commit 76621d9

11 files changed

Lines changed: 1454 additions & 180 deletions

.github/workflows/build-registry.yml

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ jobs:
2828
runs-on: ubuntu-latest
2929

3030
steps:
31-
- uses: actions/checkout@v6
31+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
32+
with:
33+
persist-credentials: false
3234

3335
- name: Install uv
3436
uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b
@@ -53,13 +55,15 @@ jobs:
5355
agent_ids: ${{ steps.list-agents.outputs.ids }}
5456

5557
steps:
56-
- uses: actions/checkout@v6
58+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
59+
with:
60+
persist-credentials: false
5761

5862
- name: Install uv
5963
uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b
6064

6165
- name: Setup Node.js
62-
uses: actions/setup-node@v6
66+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
6367
with:
6468
node-version: "lts/*"
6569

@@ -77,7 +81,7 @@ jobs:
7781

7882
- name: Upload artifacts
7983
if: (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && github.ref == 'refs/heads/main'
80-
uses: actions/upload-artifact@v7
84+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
8185
with:
8286
name: registry
8387
path: dist/
@@ -93,13 +97,15 @@ jobs:
9397
matrix:
9498
agent_id: ${{ fromJson(needs.build.outputs.agent_ids) }}
9599
steps:
96-
- uses: actions/checkout@v6
100+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
101+
with:
102+
persist-credentials: false
97103

98104
- name: Install uv
99105
uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b
100106

101107
- name: Setup Node.js
102-
uses: actions/setup-node@v6
108+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
103109
with:
104110
node-version: "lts/*"
105111

@@ -117,10 +123,12 @@ jobs:
117123
S3_PREFIX: registry/v1
118124

119125
steps:
120-
- uses: actions/checkout@v6
126+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
127+
with:
128+
persist-credentials: false
121129

122130
- name: Download artifacts
123-
uses: actions/download-artifact@v8
131+
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
124132
with:
125133
name: registry
126134
path: dist/
@@ -167,15 +175,16 @@ jobs:
167175
if: always() && !failure() && !cancelled() && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && github.ref == 'refs/heads/main'
168176
runs-on: ubuntu-latest
169177
permissions:
170-
contents: write
178+
contents: read
171179

172180
steps:
173-
- uses: actions/checkout@v6
181+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
174182
with:
175183
fetch-depth: 0
184+
persist-credentials: false
176185

177186
- name: Download artifacts
178-
uses: actions/download-artifact@v8
187+
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
179188
with:
180189
name: registry
181190
path: dist/
@@ -204,9 +213,12 @@ jobs:
204213
NOTES="- Initial release"
205214
fi
206215
207-
echo "notes<<EOF" >> "$GITHUB_OUTPUT"
208-
echo "$NOTES" >> "$GITHUB_OUTPUT"
209-
echo "EOF" >> "$GITHUB_OUTPUT"
216+
NOTES_DELIMITER="$(uuidgen)"
217+
{
218+
echo "notes<<$NOTES_DELIMITER"
219+
printf '%s\n' "$NOTES"
220+
echo "$NOTES_DELIMITER"
221+
} >> "$GITHUB_OUTPUT"
210222
env:
211223
GH_TOKEN: ${{ steps.generate-token.outputs.token }}
212224

.github/workflows/build_registry.py

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33

44
import argparse
55
import copy
6+
import ipaddress
67
import json
78
import os
89
import re
10+
import socket
911
import sys
1012
import urllib.error
13+
import urllib.parse
1114
import urllib.request
1215
import xml.etree.ElementTree as ET
1316
from pathlib import Path
@@ -54,22 +57,96 @@
5457
"true",
5558
"yes",
5659
)
60+
MAX_URL_REDIRECTS = 5
5761

5862

59-
def url_exists(url: str, method: str = "HEAD", retries: int = 3) -> bool:
63+
class NoRedirectHandler(urllib.request.HTTPRedirectHandler):
64+
"""Disable urllib's implicit redirects so each target can be validated first."""
65+
66+
def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: ANN001
67+
return None
68+
69+
70+
URL_OPENER = urllib.request.build_opener(NoRedirectHandler)
71+
72+
73+
class UrlSafetyResolutionError(Exception):
74+
"""Raised when URL safety DNS resolution fails before the request attempt."""
75+
76+
77+
def is_public_https_url(url: str, retry_dns_errors: bool = False) -> bool:
78+
"""Return whether URL validation may safely request this target."""
79+
parsed = urllib.parse.urlparse(url)
80+
if parsed.scheme != "https" or not parsed.hostname:
81+
return False
82+
83+
hostname = parsed.hostname.strip().lower()
84+
if hostname in {"localhost", "localhost.localdomain"} or hostname.endswith(".localhost"):
85+
return False
86+
87+
try:
88+
addresses = socket.getaddrinfo(hostname, parsed.port or 443, type=socket.SOCK_STREAM)
89+
except OSError as exc:
90+
if retry_dns_errors:
91+
raise UrlSafetyResolutionError from exc
92+
return False
93+
94+
for *_, sockaddr in addresses:
95+
try:
96+
ip = ipaddress.ip_address(sockaddr[0])
97+
except ValueError:
98+
return False
99+
if not ip.is_global or ip.is_multicast:
100+
return False
101+
102+
return bool(addresses)
103+
104+
105+
def url_exists(
106+
url: str,
107+
method: str = "HEAD",
108+
retries: int = 3,
109+
redirects_remaining: int = MAX_URL_REDIRECTS,
110+
) -> bool:
60111
"""Check if a URL exists using HEAD or GET request with retries."""
61112
import time
62113

63114
for attempt in range(retries):
64115
try:
116+
if not is_public_https_url(url, retry_dns_errors=True):
117+
return False
65118
req = urllib.request.Request(url, method=method)
66119
req.add_header("User-Agent", "ACP-Registry-Validator/1.0")
67-
with urllib.request.urlopen(req, timeout=15) as response:
120+
with URL_OPENER.open(req, timeout=15) as response:
68121
return response.status in (200, 301, 302)
122+
except UrlSafetyResolutionError:
123+
if attempt < retries - 1:
124+
time.sleep(2**attempt)
125+
continue
126+
return False
69127
except urllib.error.HTTPError as e:
128+
if e.code in (301, 302, 303, 307, 308):
129+
if redirects_remaining <= 0:
130+
return False
131+
location = e.headers.get("Location")
132+
if not location:
133+
return False
134+
next_url = urllib.parse.urljoin(url, location)
135+
next_method = "GET" if e.code == 303 else method
136+
return url_exists(
137+
next_url,
138+
method=next_method,
139+
retries=retries,
140+
redirects_remaining=redirects_remaining - 1,
141+
)
70142
# Some servers don't support HEAD, try GET
71143
if method == "HEAD" and e.code in (403, 405):
72-
return url_exists(url, method="GET", retries=retries - attempt)
144+
return url_exists(
145+
url,
146+
method="GET",
147+
retries=retries - attempt,
148+
redirects_remaining=redirects_remaining,
149+
)
73150
if attempt < retries - 1 and e.code in (429, 500, 502, 503, 504):
74151
time.sleep(2**attempt)
75152
continue

.github/workflows/client.py

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,30 @@
1313
from dataclasses import dataclass, field
1414
from pathlib import Path
1515

16+
from registry_utils import sanitize_agent_env, subprocess_group_kwargs, terminate_process_group
17+
18+
AGENT_ENV_PASSTHROUGH = {
19+
"CI",
20+
"COMSPEC",
21+
"NPM_CONFIG_CACHE",
22+
"NODE_EXTRA_CA_CERTS",
23+
"PATH",
24+
"PATHEXT",
25+
"PYTHON_KEYRING_BACKEND",
26+
"PYTHON_KEYRING_DISABLED",
27+
"REQUESTS_CA_BUNDLE",
28+
"SSL_CERT_DIR",
29+
"SSL_CERT_FILE",
30+
"SystemRoot",
31+
"TMP",
32+
"TMPDIR",
33+
"TEMP",
34+
"UV_CACHE_DIR",
35+
"WINDIR",
36+
"XDG_CACHE_HOME",
37+
"XDG_CONFIG_HOME",
38+
}
39+
1640

1741
@dataclass
1842
class AuthMethod:
@@ -154,6 +178,8 @@ def run_auth_check(
154178
cwd: Path,
155179
env: dict[str, str] | None = None,
156180
timeout: float = 60.0,
181+
home_dir: Path | None = None,
182+
prepend_path: list[str] | None = None,
157183
) -> AuthCheckResult:
158184
"""Verify an agent supports ACP authentication.
159185
@@ -166,16 +192,25 @@ def run_auth_check(
166192
Returns:
167193
AuthCheckResult with success status and auth methods
168194
"""
169-
# Build isolated environment
170-
full_env = os.environ.copy()
195+
# Build isolated environment without leaking GitHub Actions credentials or workspace paths.
196+
full_env = {
197+
name: value
198+
for name in AGENT_ENV_PASSTHROUGH
199+
if (value := os.environ.get(name)) not in (None, "")
200+
}
171201
full_env["TERM"] = "dumb"
172-
if env:
173-
full_env.update(env)
202+
full_env.update(sanitize_agent_env(env))
174203

175204
# Use a temporary directory as HOME if not specified
176-
if "HOME" not in (env or {}):
177-
sandbox_home = tempfile.mkdtemp(prefix="acp-auth-check-")
178-
full_env["HOME"] = sandbox_home
205+
if home_dir is None:
206+
home_dir = Path(tempfile.mkdtemp(prefix="acp-auth-check-"))
207+
else:
208+
home_dir.mkdir(parents=True, exist_ok=True)
209+
full_env["HOME"] = str(home_dir)
210+
211+
if prepend_path:
212+
base_path = full_env.get("PATH", os.environ.get("PATH", ""))
213+
full_env["PATH"] = os.pathsep.join([*prepend_path, base_path])
179214

180215
proc = None
181216
t0 = time.monotonic()
@@ -195,6 +230,7 @@ def run_auth_check(
195230
stderr=subprocess.PIPE,
196231
text=True,
197232
bufsize=0,
233+
**subprocess_group_kwargs(),
198234
)
199235

200236
# Send initialize request with capabilities
@@ -281,8 +317,4 @@ def run_auth_check(
281317
)
282318
finally:
283319
if proc:
284-
proc.terminate()
285-
try:
286-
proc.wait(timeout=2)
287-
except subprocess.TimeoutExpired:
288-
proc.kill()
320+
terminate_process_group(proc)

0 commit comments

Comments
 (0)