Skip to content

Commit bc2dd7e

Browse files
committed
fix: close link audit security gaps
1 parent 4ec4c0f commit bc2dd7e

5 files changed

Lines changed: 190 additions & 46 deletions

File tree

.github/workflows/link-audit.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ jobs:
1818
timeout-minutes: 30
1919
steps:
2020
- name: Check out repository
21-
uses: actions/checkout@v7
21+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
2222

2323
- name: Set up Python
24-
uses: actions/setup-python@v7
24+
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
2525
with:
2626
python-version: "3.12"
2727
cache: pip
@@ -37,7 +37,7 @@ jobs:
3737

3838
- name: Upload link report
3939
if: ${{ always() }}
40-
uses: actions/upload-artifact@v7
40+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
4141
with:
4242
name: external-link-report-${{ github.run_id }}
4343
path: reports/link-check.json

.github/workflows/validate.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ jobs:
1919
timeout-minutes: 20
2020
steps:
2121
- name: Check out repository
22-
uses: actions/checkout@v7
22+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
2323

2424
- name: Set up Python
25-
uses: actions/setup-python@v7
25+
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
2626
with:
2727
python-version: "3.12"
2828
cache: pip
@@ -37,7 +37,7 @@ jobs:
3737
run: python tools/validate_catalog.py
3838

3939
- name: Set up Ruby and Bundler
40-
uses: ruby/setup-ruby@v1
40+
uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0
4141
with:
4242
bundler-cache: true
4343

docs/CURATION_POLICY.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,12 @@ Internal validation runs on every pull request. External-link fetching runs only
104104
on the scheduled or manually dispatched GitHub Actions workflow.
105105

106106
A 404 or 410 from the canonical resource is strong removal evidence. A 403, 429,
107-
timeout, DNS failure, or transient 5xx is a review-needed result, not proof that
108-
the resource is gone. Retry with rate limits and record the observation date.
109-
Link checks must refuse private, loopback, link-local, and cloud metadata targets,
110-
including redirect destinations.
107+
timeout, or transient 5xx is a review-needed result, not proof that the resource
108+
is gone. DNS, connection, TLS, invalid-URL, and redirect-protocol failures make
109+
the automated audit fail, but they are still not enough on their own to remove a
110+
resource. Retry with rate limits and record the observation date. Link checks
111+
must refuse private, loopback, link-local, multicast, cloud-platform, and cloud
112+
metadata targets, including redirect destinations.
111113

112114
## Review cadence and removal
113115

tests/test_check_links.py

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
ThreadLocalSessions,
1818
UnsafeTarget,
1919
build_report,
20+
build_session,
2021
classify_status,
2122
exit_code_for_report,
2223
select_links,
@@ -84,16 +85,30 @@ def test_status_classification(status_code, expected) -> None:
8485

8586
def test_redirect_history_is_preserved() -> None:
8687
hop = FakeResponse(301, "https://example.com/old", headers={"Location": "/docs"})
87-
response = FakeResponse(200, "https://example.com/docs", history=[hop])
88-
session = FakeSession(response)
88+
response = FakeResponse(200, "https://example.com/docs")
89+
90+
class RedirectSession(FakeSession):
91+
def __init__(self):
92+
super().__init__(hop)
93+
self.responses = iter([hop, response])
94+
95+
def head(self, url, **kwargs):
96+
self.calls.append(("HEAD", url, kwargs))
97+
return next(self.responses)
98+
99+
session = RedirectSession()
89100
checker = LinkChecker(
90101
guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
91102
)
92-
result = checker.check_one(link())
103+
redirect_link = CatalogLink(
104+
"docs", "foundations", "Docs", "https://example.com/old"
105+
)
106+
result = checker.check_one(redirect_link)
93107
assert result.status == "redirect"
94108
assert result.history == [
95109
{"status_code": 301, "url": "https://example.com/old", "location": "/docs"}
96110
]
111+
assert hop.closed is True
97112

98113

99114
def test_head_failure_falls_back_to_streaming_get_and_confirms_404() -> None:
@@ -124,6 +139,10 @@ def test_transient_and_access_denied_statuses_need_review(status_code) -> None:
124139
"http://169.254.169.254/latest/meta-data/",
125140
"http://100.100.100.200/latest/meta-data/",
126141
"http://metadata.google.internal/computeMetadata/v1/",
142+
"https://168.63.129.16/",
143+
"https://224.0.0.1/",
144+
"https://[64:ff9b::7f00:1]/",
145+
"https://[::ffff:93.184.216.34]/",
127146
],
128147
)
129148
def test_literal_local_and_metadata_targets_are_blocked(url) -> None:
@@ -177,6 +196,24 @@ def connection_from_host(self, **_kwargs):
177196
assert resolved_hosts == ["example.com", "internal.example"]
178197

179198

199+
def test_checker_blocks_https_redirect_downgrade() -> None:
200+
hop = FakeResponse(
201+
302,
202+
"https://example.com/start",
203+
headers={"Location": "http://example.com/docs"},
204+
)
205+
session = FakeSession(hop)
206+
checker = LinkChecker(
207+
guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
208+
)
209+
210+
result = checker.check_one(link())
211+
212+
assert result.status == "blocked"
213+
assert "may not downgrade" in (result.error or "")
214+
assert hop.closed is True
215+
216+
180217
def test_thread_local_sessions_are_not_shared_between_workers() -> None:
181218
created = []
182219
barrier = threading.Barrier(2)
@@ -265,6 +302,55 @@ def head(self, url, **kwargs):
265302
assert "programming defect" in (result.error or "")
266303

267304

305+
@pytest.mark.parametrize(
306+
"exception",
307+
[
308+
requests.exceptions.ConnectionError("connection failed"),
309+
requests.exceptions.SSLError("certificate failed"),
310+
requests.exceptions.TooManyRedirects("redirect loop"),
311+
requests.exceptions.InvalidURL("invalid redirect"),
312+
],
313+
)
314+
def test_terminal_request_failures_are_fatal(exception) -> None:
315+
class BrokenSession(FakeSession):
316+
def head(self, url, **kwargs):
317+
raise exception
318+
319+
checker = LinkChecker(
320+
guard=guard_for(),
321+
session_factory=lambda: BrokenSession(FakeResponse(200)),
322+
workers=1,
323+
min_interval=0,
324+
)
325+
326+
assert checker.check_one(link()).status == "error"
327+
328+
329+
def test_timeout_remains_review_needed() -> None:
330+
class SlowSession(FakeSession):
331+
def head(self, url, **kwargs):
332+
raise requests.Timeout("timed out")
333+
334+
checker = LinkChecker(
335+
guard=guard_for(),
336+
session_factory=lambda: SlowSession(FakeResponse(200)),
337+
workers=1,
338+
min_interval=0,
339+
)
340+
341+
assert checker.check_one(link()).status == "review"
342+
343+
344+
def test_retry_configuration_ignores_unbounded_retry_after() -> None:
345+
session = build_session(guard_for(), retries=2, backoff_factor=0.5)
346+
try:
347+
retry = session.get_adapter("https://").max_retries
348+
assert retry.respect_retry_after_header is False
349+
assert retry.backoff_max == 5.0
350+
finally:
351+
session.close()
352+
353+
268354
def test_responses_close_when_result_processing_fails(monkeypatch) -> None:
269355
head = FakeResponse(404)
270356
response = FakeResponse(200)
@@ -273,10 +359,10 @@ def test_responses_close_when_result_processing_fails(monkeypatch) -> None:
273359
guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
274360
)
275361

276-
def fail_history(_response):
362+
def fail_classification(_status_code, *, redirected):
277363
raise RuntimeError("cannot process response")
278364

279-
monkeypatch.setattr(check_links, "_history", fail_history)
365+
monkeypatch.setattr(check_links, "classify_status", fail_classification)
280366
result = checker.check_one(link())
281367

282368
assert result.status == "error"

0 commit comments

Comments
 (0)