fix(security)!: WP_ALLOW_HTTP names hosts instead of switching the rule off (3.9.5) - #23
Conversation
…le off (3.9.5)
WP_ALLOW_HTTP=1 meant 'any host'. One variable exported for one plaintext
staging box silently covered every site the agent touched afterwards -
production included, with a reusable application password on the wire. AIG
rated it High on the 3.9.4 audit and that is the right rating: the hatch I
added in 3.9.0 was scoped to the decision ('I accept http here') but
implemented as a global ('http is fine everywhere').
It now names the hosts it permits, comma-separated. A blanket value is refused
with the exact string to write instead, and an ordinary refusal names the host
you would have to allow - the hint is actionable for that site rather than for
all of them. WP_REQUIRE_HTTPS=1 still overrides the list, and stops suggesting
the hatch it overrides.
BREAKING CHANGE: WP_ALLOW_HTTP=1 no longer works. Use the hostname.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2681a3032d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| print("SECURITY WARNING: " + msg + " (WP_ALLOW_HTTP=1 set - continuing.)", file=sys.stderr) | ||
| raw = env.get("WP_ALLOW_HTTP") or "" | ||
| allowed = {item.strip().lower() for item in raw.split(",") if item.strip()} | ||
| if allowed & {"1", "true", "yes", "all", "*"}: |
There was a problem hiding this comment.
When an upgraded environment retains the legacy WP_ALLOW_HTTP=1 while also setting WP_REQUIRE_HTTPS=1, this blanket-value branch runs first and tells the operator to replace 1 with a hostname. That suggested change still cannot work because WP_REQUIRE_HTTPS overrides the allowlist, contrary to the intended actionable diagnostic in the following branch; check the strict setting first so the refusal identifies the policy that must actually be addressed.
Useful? React with 👍 / 👎.
…t cannot help With a legacy WP_ALLOW_HTTP=1 still set alongside WP_REQUIRE_HTTPS=1, the blanket-value branch fired first and told the operator to replace 1 with a hostname - advice that cannot work, because WP_REQUIRE_HTTPS overrides the allowlist either way. Strictness is checked first now, so the refusal names the setting that has to change. Same rule I applied to the named-host branch in this PR and did not carry across to the blanket one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed. With a legacy This is the same rule I applied to the named-host branch earlier in this PR and then did not carry across to the blanket one: a refusal must name the setting that actually has to change. Strictness is checked first now, with a test pinning the message for exactly that combination. 129 tests. @codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
From the ClawHub audit of 3.9.4. AIG rated this High, and the rating is right.
The hatch was scoped to a decision, implemented as a global
WP_ALLOW_HTTP=1meant any host. The decision it was meant to express is "I accept plaintext for this staging box"; what it actually did was switch the rule off for every site the agent touched afterwards — production included, with a reusable application password on the wire each time.Exported in a shell profile or a cloud environment, it would never be noticed again.
A refusal names the host you would have to allow, so the hint is actionable for that site rather than for all of them — and
WP_REQUIRE_HTTPS=1no longer suggests the hatch it overrides, which would just fail again.BREAKING:
WP_ALLOW_HTTP=1stops working. Replace it with the hostname.What I did not change, and why
The audit also flags
requirements.txtas unpinned (Low):requests>=2.32.3,<3lets separate installs resolve to artifacts that were not audited.I am leaving it a range. An exact or hash pin stops users receiving patch-level security fixes for the very dependency whose advisory is the reason for the lower bound, and this skill ships no lockfile-refresh process to compensate — a pinned
requestshere would rot. The trade is reproducibility for patchability, and it is now written intoSKILL.mdwhere the next auditor meets it rather than argued once in a PR.Tests: 124 → 128.
🤖 Generated with Claude Code