Skip to content

Add ACP option to allow HTTP for Stop Forum Spam reports - #33

Merged
bonelifer merged 2 commits into
masterfrom
sfs-allow-http
Sep 3, 2026
Merged

bonelifer merged 2 commits into
masterfrom
sfs-allow-http

Conversation

@bonelifer

Copy link
Copy Markdown
Contributor

Summary

Adds an ACP toggle, off by default, for opting into HTTP on the Stop Forum Spam report. #29 made that request HTTPS-only; this adds an escape hatch for a server that genuinely can't make outbound HTTPS requests, without weakening the default.

  • bh_sfs_allow_http config, added via migrations/v105_data.php, defaults to 0.
  • New ACP radio (Yes/No) next to the SFS API key field: SFS_ALLOW_HTTP / SFS_ALLOW_HTTP_EXPLAIN. The explain text states plainly what enabling it sends in clear text (API key, username, IP, email).
  • event/banhammer_listener.php picks http:// vs https:// based on the config value; everything else about the request (the encoding fix from Fix reflected XSS in bh_res and harden StopForumSpam transport #29) is untouched.

Depends on #29 — this branch is built on top of fix-bh-res-xss since it touches the exact same request-building code; #29 should merge first.

Test plan

  • php -l passes on every changed/added PHP file
  • Confirm the ACP setting saves and persists
  • Confirm the report actually goes out over HTTP when enabled, HTTPS when not

🤖 Generated with Claude Code

bonelifer and others added 2 commits September 3, 2026 18:22
Defaults to HTTPS (bh_sfs_allow_http = 0), matching the fix in #29.
An admin whose server can't make outbound HTTPS requests can opt into
HTTP explicitly in the ACP; the setting is off by default and the ACP
label spells out what enabling it actually sends in clear text.

Builds on the not-yet-merged fix-bh-res-xss branch (#29), which is
where the HTTPS-only SFS request this adds a toggle for was fixed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the existing SFS API key field's own SFS_CURL gating - when
cURL is missing, no SFS request is ever sent regardless of this
setting, so showing the transport choice is just noise on top of the
SFS_NEEDS_CURL message already telling the admin reporting won't work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bonelifer

Copy link
Copy Markdown
Contributor Author

(Claude, replying on William's behalf)

Two follow-up commits pushed to this branch since the PR was opened:

  • Validation fix — the new SFS_ALLOW_HTTP toggle showed unconditionally, even when SFS_CURL is false (i.e. the server can't make the SFS request at all, so the transport choice is moot). Wrapped it in the same <!-- IF SFS_CURL --> gate the existing API-key field already uses.
  • Rebase onto master — this branch was behind after Fix composer.json schema, a migration data-loss bug, and other validation findings #30 merged (which removed the now-dead SETTINGS_ERROR language key). Rebased and resolved the one conflict in language/en/banhammer_acp.php by dropping the stale re-add of that key and keeping the two new SFS keys. No functional change from the rebase itself.

PR is clean/mergeable against current master now.

@bonelifer
bonelifer merged commit 72213c9 into master Sep 3, 2026
5 checks passed
@bonelifer
bonelifer deleted the sfs-allow-http branch September 3, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant