Skip to content

v2.53.0

Choose a tag to compare

@github-actions github-actions released this 08 Sep 20:57
· 1 commit to main since this release

Security

  • allowed_users gated the admin menus and nothing else, so a site that set it was not getting what the setting looks like. The list was consulted in exactly one place, Admin::current_user_allowed(), which fronted the menu registrations, the render_*_page() calls and the two admin_post_ handlers. Every REST permission callback and every capability-gated verb decided on Capabilities::can() / require() instead, and neither read the list — so an administrator the operator deliberately excluded lost the menus and kept the entire control plane. The path was short: open devtools on any wp-admin page for the wp_rest nonce, POST /newspack-nodes/v1/auth — which needs only Capabilities::can( READ ) and clamps the granted scope to the caller's own capabilities, so an administrator gets a MANAGE-scoped signing key — then post a signed topologies save to /command and write a .tsl the next worker spawn executes. The check now lives in Capabilities::can(), the one gate every surface already funnels through, so /command, /auth, /messages/stream, /log/stream, the spawn endpoint's external path, every Service CI verb and the admin pages all inherit it from one place. It is checked AFTER the capability, keeping the property the setting was documented with — a demoted account loses access with no edit here — and it applies to an AUTHENTICATED actor only: with no login there is nothing to match, so WP-CLI without --user, worker processes and WP-Cron are never narrowed, and a populated list cannot disarm the fleet or a CLI verb. Spawn_Controller's internal HMAC path returns before it reaches can() at all, so a worker reviving a peer holds no WordPress user and needs none. A site that already sets allowed_users must add any SERVICE account that reaches it over REST — the log aggregator's hub user, an HTTP_Out credential another site holds in its Vault — or that account starts answering 401 where it used to pass. wp nodes hub-user warns about exactly that: it consults the list after setting the role and, when a populated one omits the login, names the login and the config key to add it to. It does not refuse — the operator may be editing the list next. The list read fails CLOSED inside can() rather than propagating: Config::value() raises on an invalid config file, an unreadable LOCAL_NEWSPACK_NODES_CONF or an unregistered key, and can() is what four REST permission_callbacks answer with and what Auto_Tuner_Node::fill() asks on the request path, so a throw there would be an uncaught 500 in place of a 403 and an ADR-13 violation. It refuses the role and reports the reason through Core::print_less_often() instead; the \InvalidArgumentException cap_for() raises on a broken newspack_nodes/capability_map filter is a different contract and still propagates. One narrower change rides along: a scalar allowed_users, which is a config typo rather than an absent list, is read as a one-login allowlist, where the admin gate treated any non-array as "no list" and admitted every capable user; nuclear-gyrobase fixed the identical fail-open in its own copy of this rule. Residual, and it is small: a command SESSION authenticates by HMAC rather than by WordPress user, so a key minted before the exclusion stays cryptographically valid until its TTL expires (60s..86400s, Command_Auth::bounded_ttl(), default SESSION_TTL_S) — but it buys nothing, because HTTP_In_Node::check_permission() runs can( READ ) on every /command POST before a signature is examined, so the excluded user is refused at the door on the very next request. The one window that does outlive the change is an SSE stream already open: check_permission() runs once at connect, so an established /messages/stream or /log/stream connection keeps delivering until it drops.

  • A control character in untrusted text reached an operator's terminal unrendered, and an anonymous web visitor could put one there. A request whose URL, Referer or User-Agent carried raw 0x1B bytes lands in wp-content/debug.log through any PHP notice; debug is a built-in Log_Sources entry needing no configuration, so an operator running taillog debug in wp nodes cli had those bytes fwritten verbatim to their TTY — a \033]0;…\007 retitling the window, an \033[2J clearing the screen, and a \r rewriting the line already read, so the log could show one thing and the terminal another. The same applied to a worker-set prompt, which Dumper_Node::fill() writes from an attached worker's prompt response and hands to readline unexamined. New Core::terminal_safe() now renders every C0 control character and DEL as a visible <XX> token — reverse-videoed on a real terminal, bare in a pipe, a file or a test capture — at all four places untrusted text reaches a terminal: Stdout_Node::write() (which wp nodes reqgrep writes through as well), TTY_Out_Node's redraw and write_prompt(), and the prompt TTY_In_Node installs into readline. It RENDERS rather than strips, because a stripped byte hides the attack from whoever is reading the log, and rather than refuses, because a log tail legitimately holds whatever the log holds. Newline and tab pass through and \r does not. The C1 block renders in BOTH of the encodings a terminal acts on, as the codepoint either way — <9B>, never <C2><9B>, because the codepoint is what names the control an operator has to recognize. A lone 0x80-0x9F byte is 8-bit CSI or OSC on an xterm, rxvt or screen outside UTF-8 mode — LANG=C is enough — so User-Agent: \x9B2J cleared the screen exactly as \x1B[2J did. \xC2 followed by that same byte is well-formed UTF-8 for the very same codepoint, and a UTF-8-mode terminal acts on what it decodes: VTE — GNOME Terminal, Tilix, Terminator, xfce4-terminal — dispatches U+009B from its ground state straight into CSI_ENTRY and U+009D into OSC_STRING, so \xC2\x9B2J cleared the screen too, and on VTE that two-byte form was the ONLY reachable form, since it requires UTF-8 and never sees the lone byte as a control; the scan claims that two-byte sequence ahead of the general two-byte branch that would otherwise pass it through as an ordinary character. Everything else well-formed still passes byte-identical, \xC2\xA0 one codepoint above the block included, and a 0x80-0x9F continuation byte inside any longer sequence with it. A failed render pass throws, where the (string) cast on preg_replace_callback()'s null turned a PCRE error into an empty write, dropping the very line the tail was opened to show. Rendering is the default on every message path — wp nodes reqgrep, a Dumper's line, an interpreter reply — and Stdout_Node::write_raw() is the single opt-out, for a caller composing a control sequence on purpose; the only one in the tree is Shell_Node's clear builtin, which otherwise printed <1B>[2J<1B>[H instead of clearing the screen. The prompt renders BARE at all three sites that draw it, payload text keeping reverse video on a terminal: highlighting is how the renderer marks an anomaly inside data the operator is reading, and a prompt redrawn behind every async write would flicker between two looks on readline's next forced redisplay. CLI::cli_safe(), which stripped, is gone: its three terminal-bound error messages render through the same helper.

Removed

  • BREAKING: Admin\Admin::current_user_allowed() is gone; call Capabilities::can( Capabilities::MANAGE ). With the allowlist inside can() the method was Capabilities::can( MANAGE ) under a second name, and a wrapper carrying a rule that also lives elsewhere is the drift this codebase keeps paying for. All eight substrate call sites, the bundled example-ai-newsletter, docs/writing-a-dashboard.md and docs/stability.md's frozen-surface list move with it — and Capabilities is now DECLARED on that list as item 12, since three consumers call can() directly, event-logger-nodes' admin also calls cap_for(), and every consumer service CI compiles the three role constants into its node_schema(). The reference in item 10 pointed at a frozen surface the document never named. A deleted public method cannot degrade. The substrate necessarily ships FIRST — a consumer pins the substrate tag, so the tag exists before the consumer built against it — and an older consumer calling this name fatals with Call to undefined method rather than losing a feature, which is what Partition_Node::locate_by() already cost us as an uncaught 500 on every dashboard request. One consumer calls it: newspack-intelligence, every release through 0.9.9, from three admin hooks — the Publisher Insights menu, the Settings submenu, and the dashboard enqueue on admin_enqueue_scripts, which fires on every wp-admin page. Its version_at_least( '2.25.0' ) floor does not help; the handshake passes, the plugin wires itself up, and it fatals. Intelligence is updated in the same pass and must be released beside this substrate. No other plugin in the family calls it — event-logger-nodes has a same-named method on its OWN Admin class, unrelated and untouched.