Commit 27aa882
authored
feat(dashboard): admin-only project announcements feed (#1393)
* feat(dashboard): admins-only project announcements feed
Anonymous, opt-out-by-default daily fetch of project news + security
advisories from a JSON file the maintainers publish at
https://sbpp.github.io/announcements.json. The dashboard renders the
freshest non-expired entry as a slim disclosure strip between the
stat cards and the recent-activity panels — visible only to logged-in
admins. The shape mirrors the existing telemetry tick (`Sbpp\Telemetry\Telemetry`)
and update-check cache (`system.check_version`'s
`_api_system_release_*` helpers) so the lifecycle, cache contract,
and never-fail-the-request guarantees are familiar.
Architecture
------------
Source of truth lives in this repo at `docs/public/announcements.json`
— Astro publishes everything under `docs/public/` as a static asset
at the docs site root (proven by `favicon.svg`). Maintainers ship
announcements via PRs against this file; the existing
`docs-deploy-trigger.yml` workflow lands the updated file at
`https://sbpp.github.io/announcements.json` within minutes of merge
to `main`. The starter file ships as `[]` (empty array) so the
deploy chain validates end-to-end before any real content goes out.
`web/includes/Announce/AnnouncementFetcher.php` (`Sbpp\Announce\AnnouncementFetcher`):
- `latest()` reads the cache only — never blocks on the network.
A cold-cache install renders no banner; the next render after the
shutdown hook lands the cache shows it. This is the documented
"first render after install renders no banner" behaviour.
- `tickIfDue()` is registered as a `register_shutdown_function` at
the tail of `web/init.php`, gated on a non-empty
`SB_ANNOUNCEMENTS_URL` constant. The `if (!defined(...))` guard
lets `config.php` win — `define('SB_ANNOUNCEMENTS_URL', '')` is
the documented air-gap escape hatch (covered in
`docs/src/content/docs/configuring/announcements.mdx`).
- 24h TTL gate inside `tickIfDue()` keeps the actual outbound call
to at most one per install per day regardless of request volume.
On FPM, `fastcgi_finish_request()` flushes the response BEFORE
the upstream call so the user's TCP socket closes first; non-FPM
SAPIs fall back to `ob_end_flush + flush`. The outer
`try { run(); } catch (\Throwable) {}` wrapper guarantees a
page render or JSON API call NEVER fails because the upstream
is flapping.
- Cache shape mirrors `system.check_version`'s
`_api_system_release_save_cache`: atomic tempfile +
`rename()` write under `SB_CACHE/announcements.json`, persisted
as the raw upstream body verbatim. Parsing happens at read time
so the on-disk file is byte-identical to what the upstream
served — easier to triage a "wrong content rendered" report.
- Stale-while-error: a failed upstream call leaves the previous
cache untouched; the operator keeps seeing the previous
announcement until a successful fetch overwrites it.
- Wire layer mirrors `system.check_version`'s helper: 5s timeout,
256 KiB hard cap (enforced by the stream wrapper's `length`
parameter AND re-asserted post-read so a future swap of the
read layer doesn't bypass it),
`User-Agent: SourceBans++/<ver> (announcements)`, no query
parameters, no cookies, no tracking pixels.
- Test override: `_setHttpFetcherForTests(?callable)` mirrors
`Sbpp\Servers\SourceQueryCache::setProbeOverrideForTesting`.
Production never sets it.
Surface gating
--------------
The page handler in `web/pages/page.home.php` short-circuits to
`null` for anonymous + non-admin viewers
(`$userbank->is_admin() ? AnnouncementFetcher::latest() : null`);
the `HomeDashboardView` carries `?array $announcement`; the
template (`page_dashboard.tpl`) gates the entire `<aside>` block
on truthy `$announcement` so the strip never paints for visitors
who can't act on the content. The DTO is converted to a Smarty
array so the template reads `{$announcement.title}` etc. with
global auto-escape; `body_html` is the only field with `{nofilter}`
because it's already `Sbpp\Markup\IntroRenderer` output (the only
"this is already safe HTML" exit point the panel supports — see
"`nofilter` discipline" + "Admin-authored display text" in
AGENTS.md).
The strip's chrome is a slim `<details>` disclosure styled to
match the existing `.empty-state` / card vocabulary: subtle muted
background, single-line summary with megaphone icon + title +
right-aligned date, body slot for IntroRenderer-rendered HTML,
optional "Read more" external link with the textbook
`target="_blank" rel="noopener noreferrer"` reverse-tabnabbing
guard. Dark mode flips automatically via the `var(--*)` tokens.
The chevron rotation honours the global
`prefers-reduced-motion: reduce` reset; spinner / shimmer-style
per-rule overrides aren't needed here because the disclosure is
*motion-of-state*, not essential feedback.
Trust + safety
--------------
- Markdown bodies (`body_md`) go through
`Sbpp\Markup\IntroRenderer::renderIntroText()` which wraps
league/commonmark with `html_input: 'escape'` +
`allow_unsafe_links: false`. Inline HTML is rendered as escaped
text; `javascript:` / `data:` / `vbscript:` URLs are stripped.
Same renderer the dashboard intro uses — reaching for any other
Markdown library here would re-open the #1113-class stored-XSS
vector.
- The top-level `url` field is rendered as a literal `<a href>`
so the parser additionally rejects non-`http(s)://` schemes at
validation time (defence-in-depth).
- Outbound URL is hardcoded `https://` in the default define;
operators with strict egress controls disable the fetch
entirely with `define('SB_ANNOUNCEMENTS_URL', '')` in
`config.php`. There is no in-panel toggle by design — the
feed is intentionally low-frequency + audit-friendly, so the
only sensible "off" position is the documented config.php-side
escape hatch.
- Operators audit content via the public diff history of
`docs/public/announcements.json` — every announcement that
has ever been pushed lands there before it lands on their
dashboard. The single-file shape is the load-bearing audit
property.
- No DB writes, no `:prefix_settings` rows, no migration script.
Cache file lives under `SB_CACHE`.
- The parser drops malformed entries (missing / empty / overlong
`id`, missing / empty `title`, non-string body_md, non-http
url), expired entries (`expires_at` past), and duplicates
(first-occurrence-wins on `id`). Sorts newest-first by
`published_at`; entries without it sort below dated entries
so a maintainer who forgets the field doesn't accidentally
pin their entry to the top.
Tests
-----
- `web/tests/integration/AnnouncementFetcherTest.php` — 18 cases
covering cache shape (atomic write, no tempfile leak), TTL
gate (cold / fresh / stale), stale-while-error, body-cap
enforcement (oversized rejected), malformed JSON, missing
required fields, expired-entry filter, newest-by-published_at
sort, undated entries sort last, duplicate-id de-dup, non-http
URL rejection, IntroRenderer integration (literal `<script>`
in body_md is escaped), verbatim-cache persistence, and
ISO + integer timestamp parsing.
- `web/tests/integration/HomeDashboardAnnouncementTest.php` —
4 cases on the page handler's surface gate. Process-isolated
render with a stub Smarty (mirrors `Php82DeprecationsTest`'s
shape) so the `HomeDashboardView`'s `announcement` property
gets captured without rendering the actual `.tpl`. Three tests
cover the gate (admin + populated → array, anonymous +
populated → null, admin + cold → null); the fourth pins the
View DTO contract via `ReflectionClass`.
- `web/tests/e2e/specs/flows/dashboard-announcement.spec.ts` —
end-to-end coverage. Seeds the cache via a PHP shim
(`web/tests/e2e/scripts/seed-announcements-e2e.php` +
`seedAnnouncementsE2e` / `clearAnnouncementsCacheE2e` in
`fixtures/db.ts`) so the spec is deterministic without making
the real https://sbpp.github.io fetch. Asserts the strip
mounts when populated, the disclosure expands, the body
paints the IntroRenderer-rendered HTML, the external link
carries `rel="noopener noreferrer"`, and axe-core passes.
An anonymous-storage-state `describe` block asserts the
strip never paints for logged-out visitors.
Docs
----
- `docs/src/content/docs/configuring/announcements.mdx` — new
operator-facing doc explaining the lifecycle, what gets
fetched, what isn't sent (no cookies, no query params), the
rationale for opt-out by default, and the air-gap escape
hatch. Filed under a new "Configuring" sidebar group in
`docs/astro.config.mjs`.
- `AGENTS.md` — new "Project announcements feed" Conventions
block (full lifecycle / cache shape / wire layer / Markdown
rendering / test-override contract), two new "Where to find
what" rows (publish/amend an announcement; build/extend the
feed wiring), one new "Keep the docs in sync" row, and four
new Anti-patterns (in-panel toggle; reaching for
`league/commonmark` directly; editing announcements outside
the source-of-truth file; removing the eager
`register_shutdown_function` call).
- `ARCHITECTURE.md` — new directory-layout entry under "Web
panel → Directory layout" for `web/includes/Announce/`.
Quality gates
-------------
PHPStan, PHPUnit, ts-check, api-contract all green locally.
PHPStan needed one source-side fix during the run: an
unused `CONNECT_TIMEOUT_SECONDS` constant flagged by the dba
plugin's `classConstant.unused` rule. The constant was
reserved for a future cURL-based reshape (`CURLOPT_CONNECTTIMEOUT`
vs `CURLOPT_TIMEOUT`); since the current `file_get_contents` +
stream-context implementation only honours a single timeout,
the constant came out and the docblock on `TOTAL_TIMEOUT_SECONDS`
documents the future split. No baseline entries added.
* fix(announcements): defence-in-depth scheme guard + bounded cache load
Two adversarial-review hardenings on the daily project-announcements
fetcher. Both close gaps where a misconfigured config.php (or a
hostile actor with write access to it) could turn the cache path
into something it shouldn't be.
Scheme guard in `resolveUpstreamUrl`
------------------------------------
`SB_ANNOUNCEMENTS_URL` is operator-overridable via `config.php`. The
pre-fix resolver passed the URL through to `file_get_contents`
verbatim, which honours every stream wrapper PHP ships
(`file://`, `php://`, `phar://`, `data://`, `ftp://`, `gopher://`,
…). A typo, an SSRF gadget, or a compromised config could land:
- `file:///etc/passwd` → arbitrary local file read into the cache,
then re-rendered to admins on the dashboard.
- `php://filter/read=convert.base64-encode/resource=…` → arbitrary
file disclosure dressed as base64 payload.
- `phar://` → unmarshalling-side attack on PHP < 8.0 paths (we're
PHP 8.5 so this is moot, but cheap belt-and-braces).
`resolveUpstreamUrl()` now `preg_match('~^https?://~i', …)`s the
configured value and short-circuits anything else to the empty-
string air-gap branch. The constant itself is still
operator-overridable; the gap is closed at the resolver, which is
the single funnel every code path goes through.
Side-effect of the rewrite: pulled the value through `mixed` (via
`/** @var mixed */ + is_string`) so PHPStan stops narrowing to the
init.php compile-time literal. With the literal narrowing,
`$url === ''` was flagged as `identical.alwaysFalse`; with `mixed`,
both the empty-string and the non-empty arms stay reachable from
the analyser's perspective (which is the actual contract — operators
do redefine the constant in `config.php`).
Bounded cache load in `loadCachedEntries`
-----------------------------------------
Pre-fix the cache reader called `file_get_contents($file)` with no
`maxlen`, so a hand-edited / hostile / accidentally-bloated cache
file (`SB_CACHE/announcements.json`) was loaded fully into memory
BEFORE the body-size assertion ran. A 1 GiB JSON blob would OOM the
worker; the asserter would never fire.
The fetcher's wire side already bounded reads at
`MAX_BODY_BYTES + 1`; the cache reader now does the same so the
parse boundary is identical regardless of whether bytes got onto
disk through `tickIfDue()` or some other path. The post-read size
check stays in place as defence-in-depth — anything strictly larger
than the cap means the source served > 256 KiB; reject silently and
let the next tick overwrite with a fresh upstream body.
Test override hook
------------------
The constant is write-once at runtime, so the existing
`testEmptyAirGapUrlShortCircuitsTheFetcher` test was a no-op stub
(`assertSame('', '')`) — it couldn't actually drive the air-gap
branch. Added `_setUpstreamUrlForTests(?string $url)` mirroring the
existing `_setHttpFetcherForTests` shape; tests can now exercise
the empty-URL branch AND the new scheme-guard branch deterministically
(see the paired test commit on top of this one).
AGENTS.md
---------
- Wire layer: corrected the timeout description from
"5s timeout, 3s connect timeout" to "5s combined connect+read
timeout" — PHP's stream wrapper exposes a single `timeout` knob
covering both legs (the original copy was a copy-paste from the
telemetry block which uses cURL with split connect / total).
- Documented the new scheme guard alongside the air-gap hatch in
the "Wire layer" + "Where to find what" rows.
- Documented the bounded cache load in the "Where to find what"
row's cache-shape blurb so future readers know why the read is
capped at MAX_BODY_BYTES + 1 in two places (fetch and load).
* fix(announcements): serialize e2e spec to avoid cache-file race
Every test in `dashboard-announcement.spec.ts` mutates the SAME
`SB_CACHE/announcements.json` file: each `beforeEach` seeds the
cache via the shell-out helper, the test renders the dashboard,
the `afterEach` clears the file. The dev container has one panel
root and one cache directory — the cache file is a process-global
singleton from the panel's perspective.
The suite default is `fullyParallel: true` plus `workers: undefined`,
so non-CI runs spawn multiple workers and sibling tests in this
file race against each other:
- Worker A's `renders the strip` seeds the cache, navigates,
expects the strip to be visible.
- Worker B's `strip is absent when the cache is cleared mid-test`
seeds, clears, expects no strip.
If B's `clearAnnouncementsCacheE2e()` lands between A's seed and
A's first paint, A renders an empty strip and the test fails —
exactly the symptom seen in adversarial review (intermittent fail
locally, passes in isolation).
File-scope `test.describe.configure({ mode: 'serial' })` pins all
tests in this spec to a single worker so the cache file only ever
has one owner at a time. Other specs continue to run in parallel
— only this file's intra-file ordering is constrained. Same shape
`_screenshots.spec.ts` uses for its per-route DB seed/restore loop.
CI continues to enforce `workers: 1` globally (the AGENTS.md
"Playwright E2E specifics" note documents why), so this only
matters for local runs — but the gate is `failOnFlakyTests: true`,
which would have surfaced this on the next CI flake.
* test(announcements): cover air-gap and scheme-guard branches
Replaces the placeholder `testEmptyAirGapUrlShortCircuitsTheFetcher`
test (a `assertSame('', '')` no-op with a docstring explaining why
it can't actually test anything) with real coverage of every
`resolveUpstreamUrl()` arm. Pairs with the `_setUpstreamUrlForTests`
override added in the previous commit.
New tests:
- `testEmptyAirGapUrlShortCircuitsTheFetcher` — drives the empty-
URL branch via the override. The HTTP-fetcher override is a
call-counter that fails the test if invoked, AND the cache file
is asserted to not exist after the tick. The pre-fix stub
asserted neither.
- `testNonHttpUrlSchemeShortCircuitsTheFetcher` — six-row data
provider covering the stream wrappers `file_get_contents`
honours by default: `file://`, `php://`, `phar://`, `data://`,
`ftp://`, `gopher://`. Each must short-circuit to the air-gap
branch (no fetcher invocation, no cache write).
- `testHttpsUrlIsAccepted` / `testHttpUrlIsAccepted` — positive
arm. A self-hosted mirror at `https://mirror.example.com/…`
flows through to the fetcher unchanged; same for plain `http://`.
Pinning both shapes so a future tightening of the regex (e.g.
`https://`-only) doesn't silently break self-hosters on intranet
mirrors without TLS.
Also: switched the data-provider declaration from the legacy
`@dataProvider` annotation to the PHPUnit 13 `#[DataProvider]`
attribute. The annotation form still parses but PHPUnit 13's
attribute discovery is the supported shape going forward (and
the docblock form silently no-ops on `--strict-coverage` etc.).
Updated setUp/tearDown to clear the URL override too, mirroring
the existing httpFetcher reset pattern.
26 tests / 57 assertions, all green.
* docs(updating): mention always-on announcements feed in the v1.8→2.0 page
The 1.8 → 2.0 upgrade page lists the three things to know before
running the panel updater (PHP floor, theme reset, anonymous
telemetry). The announcements feed is a new always-on outbound
channel that ships in the same release; upgraders deserve to find
it on the same page they're already reading.
Adds:
- A fourth bullet to the "things to know" list at the top.
- A new `## Project announcements feed` section after the existing
`## Anonymous telemetry` section, structured the same way:
what's sent (User-Agent only, no instance ID, no cookies, no
Referer, no query string), what's never sent (audit-trail
anchored to the file in this repo), how to opt out
(`define('SB_ANNOUNCEMENTS_URL', '')` in `config.php`), and
performance impact (zero on PHP-FPM via `register_shutdown_function`,
same shape as telemetry).
The opt-out instructions link to the existing
`docs/configuring/announcements/` page (added in the worker's
commit) for the full reference; the upgrade page is intentionally
a tighter "operator-needs-to-know" subset.
Calls out that the two channels (telemetry, announcements) are
intentionally separate: telemetry is write-only push, announcements
is read-only pull. Neither can affect the other, and opting out of
one leaves the other running. This is the question every
privacy-conscious operator will ask after seeing two outbound
channels in the same release; the docs answer it once, in the place
they're already reading.1 parent 07d8936 commit 27aa882
18 files changed
Lines changed: 2391 additions & 1 deletion
File tree
- docs
- public
- src/content/docs
- configuring
- updating
- web
- includes
- Announce
- View
- pages
- tests
- e2e
- fixtures
- scripts
- specs/flows
- integration
- themes/default
- css
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| 101 | + | |
101 | 102 | | |
102 | 103 | | |
103 | 104 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
128 | 134 | | |
129 | 135 | | |
130 | 136 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| |||
177 | 179 | | |
178 | 180 | | |
179 | 181 | | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
0 commit comments