Skip to content

BOGO Select for WooCommerce 2.3.9

Choose a tag to compare

@johnjanney johnjanney released this 17 Aug 21:17
· 4 commits to main since this release
f18c8d7

Install: download bogo-select-2.3.9.zip below, then WordPress → Plugins → Add New → Upload Plugin.

[2.3.9] — 2026-08-17

A price the browser stopped parsing twice, and the release machinery that had
been waiting for something to ride out with.

The shipped change is one CodeQL finding. A variation's price travelled to the
browser as markup on an attribute and the selector wrote it back with
innerHTML, so the figure was parsed into HTML a second time — past the filter
the server had already run over it. Nothing a customer can type reaches that
markup, so this is a PATCH and not an emergency; what it corrects is that the
same string had a filtered path and an unfiltered one and only one was checked.

Everything else here is the release process and its safety nets, accumulated
since 2.3.8 and held back because tooling does not bump a version on its own
(BRIEF.md §8.1). This is the release they ride out with.

Added

  • A check that the suite would object to a real defect, and the hole it
    found on its first run. bin/verify-tests.sh reintroduces eight defects this
    plugin actually had — the Buy list ignoring a variation's own ID, a gift line
    counting toward another gift, a reversed schedule saving anyway, a date read
    with trailing junk, a Shop Manager unable to save, the summary counting list
    entries, a search loading every candidate twice, an array becoming product 1 —
    and requires the unit suite to fail on each.

    It exists because a green suite says the tests agree with the code, not that
    they would object to different code, and those are separate claims. v2.3.1
    shipped a browser assertion that passed on a negative true either way, for two
    releases.

    Seven were caught. The eighth survived: the v2.3.7 fix had no test. That
    release refused a non-scalar where a product ID belongs, and reverting it
    changed nothing the suite noticed — behaviour described in a changelog and
    guarded by nothing. Four tests cover it now.

    It runs as its own CI job on one PHP version, since whether a test notices a
    defect does not vary by interpreter.

  • The same check for the browser assertions, which is the half that matters
    more: the browser layer is where the vacuous assertion actually shipped.
    bin/verify-browser-tests.sh runs inside the integration job, reusing the
    stack it has already built, and copies each mutated file straight into the
    installed plugin — standing up WordPress per mutation is not affordable.

    Five defects: the phone layout's touch targets shrinking back to what they
    were before 2.3.5, a gift card ceasing to be a row, the chooser's listeners
    leaving the document as they were before 2.2.1, and the two settings-screen
    refusals M-01 and M-02 turn on.

    Every target test runs before its mutation and must pass. Without that a
    broken stack would report every mutation as caught, which is the most
    flattering possible way for a check like this to be useless.

  • A release gate that refuses to publish a tag CI did not pass.
    bin/verify-ci.sh resolves a tag to its commit, checks the tag on origin
    points at the same commit, finds the CI run for that SHA, waits if it is
    still going, and exits non-zero unless it concluded success. It is now step
    2 of BRIEF.md §8.4, between pushing the tag and publishing the release.

    Two tags had already gone out red. v2.3.1's integration lanes failed on an
    assertion introduced in the same release; v2.3.8's coding-standard job failed
    on a docblock. Both were found afterwards, by hand, and only because someone
    went looking. The script was checked against both: it refuses each and names
    the jobs that failed.

    .github/workflows/release-gate.yml runs the same script on
    release: published, so a release cut by any route — the web UI, a direct
    gh release create, someone else's hands — is checked even though the
    process step was skipped. It reports and does not unpublish: v2.3.8's red run
    was a docblock alignment in a comment, and withdrawing a sound archive over
    that would have been the larger harm. What to do about a red release is a
    judgement, and the workflow's job is to make sure it is offered rather than
    missed.

    The manual check it replaces was worse than no check. It asked for "the most
    recent run" moments after pushing, which is frequently the previous
    commit's — so it could answer green for a commit that was never tested, in a
    form that read as verification. Runs are matched by SHA and by nothing else,
    and a commit with no run at all is refused rather than assumed fine, which is
    what v1.0.0 would be: tagged before the pipeline existed.

  • The 2.3.x line summarised in Simplified Technical English, and kept out of
    the archive.
    RELEASE-SUMMARY-2.3.x-STE.md restates 2.3.1 to 2.3.8 in
    ASD-STE100: one section per release, a limited vocabulary, and short
    sentences. The entries below are each written at the length their release
    deserved, which is the right form for the record and the wrong one for a
    reader who wants to know what eight patch releases did. It adds nothing to
    them, and where the two disagree this file is right — including 2.3.8's
    correction of the two claims 2.3.7 shipped with, which the summary repeats as
    a correction rather than quietly fixing in its 2.3.7 section.

    It does not ship. 'RELEASE-SUMMARY*.md' is in PACKAGE_EXCLUDES beside
    CODEX-REVIEW*.md, because the archive already carries this changelog, and a
    second shorter account of the same releases gives a store two documents that
    can disagree — with the summary the one that goes stale first. A pattern
    rather than a filename, so the exclusion holds for the next summary without
    anyone remembering it.

    It would otherwise have shipped, and that is the manifest working as written
    rather than an oversight. The list names what stays out and packages
    everything else, so a new runtime file is packaged and verified without
    being named anywhere — which is the property that stopped the build and the
    verifier drifting apart. The cost is the other direction: a document meant for
    the repository alone has to be excluded on purpose. Checked by reading the
    list rather than by trusting the edit — the manifest returns the same 23 files
    as before the summary landed, and bin/verify-zip-check.sh still catches all
    six ways an archive can be wrong.

Fixed

  • CI was red on the v2.3.8 tag, from a docblock the coding standard would not
    accept.
    The level-9 work spelled a variation option's shape out inline, and
    at 85 characters the standard's parameter alignment rule wanted the next line
    indented to match it. The shape is named once on the class now and referred to
    by that name in the three places that used it, which is shorter to read than
    either the inline type or the alignment it demanded.

    It reached the tag because the release checks ran phpcs | tail -3, and a
    summary report's last three lines are a separator and a timing — the findings
    above them were cut off, so a failing run read as a passing one. The same
    shape of mistake as the archive that reported "87 runtime files verified".
    Checks are read by exit code now, not by the tail of their output.

    No runtime file changed and the published 2.3.8 archive is unaffected.

  • The archive check now reads every entry in the archive
    (CODEX-REVIEW.md L-03, the last of that finding's recommendations). It
    compared .php, .js, and .css and nothing else, so of the 28 entries in
    the 2.3.8 archive it looked at 14. The changelog, the brief, the decision
    record, and the licence shipped in every release unread — a stale or
    substituted document was outside what the gate could see, and the gate said
    "matches the worktree" regardless.

    Three questions are asked of each entry now, rather than one of some of them:
    it must be a file the worktree has, with an identical SHA-256, that the build
    was meant to ship. The last of those is new and catches what the other two
    cannot — a node_modules/ or a composer.lock inside an archive is a defect
    even when it matches the worktree perfectly, which is exactly how the 2.3.5
    archive passed.

    Entries are read from the archive with unzip -Z1 rather than by walking the
    unpacked tree, because a file installed outside bogo-select/ would unpack
    outside the directory a walk of that directory can reach.

  • The build and the verifier now read one list. bin/package-manifest.sh
    holds the exclusions; build-zip.sh copies what it returns and
    verify-zip.sh checks the archive against the same thing. They used to keep
    a list each — one in rsync --exclude flags, one in a find prune, and a
    third in the no-rsync fallback — which is why node_modules could be added to
    the build and missed by the check.

    The build is an allowlist now: it copies the files the manifest returns
    instead of copying everything and deleting afterwards, and the -x '.*'
    filter at zip time is gone, since a second filter after the list is decided is
    another place for the two to disagree. The 2.3.8 archive rebuilt this way
    contains the same 28 entries as the published one.

    Both directions were checked against a deliberately wrong archive: drifted
    CHANGELOG.md and LICENSE are reported stale, a removed file missing, a
    smuggled composer.lock and node_modules/ excluded, an unknown file extra,
    and an entry outside bogo-select/ out of tree.

  • The parity gate is now required to object, the way the suites are.
    bin/verify-zip-check.sh builds real archives in a temporary sandbox, breaks
    each one of those six ways, and requires verify-zip.sh to fail and to name
    the reason
    — a non-zero exit for the wrong reason is not a catch, since a
    gate that rejects everything is as useless as one that rejects nothing and
    reads the same from outside. It runs in CI beside the parity check, and never
    writes to the checkout.

    Run against the previous verify-zip.sh, all six survive. That is the
    finding stated as a test rather than as a paragraph: every one of those
    archives would have been published, and the gate would have said "matches the
    worktree" each time.

    This is the third check here found to be narrower than its report — after the
    browser assertion that passed on a negative true either way, and phpcs | tail -3. The pattern is the same every time and is invisible from the
    inside: a file not read looks exactly like a file that is fine. The only
    answer is to break something on purpose and require the objection.

Security

  • A variation's price no longer travels to the browser as markup on an
    attribute
    (CodeQL js/xss-through-dom, CWE-79). The card carried each
    option's price HTML in data-price and the selector wrote it into the page
    with innerHTML, so the figure was parsed into HTML a second time — the first
    parse being the one the server had already filtered with wp_kses_post(). The
    attribute path never ran that filter, which made the script the one place the
    plugin's own trust boundary did not hold.

    Every option's figure is rendered into the card instead, filtered once each,
    and the selector now only changes which one is hidden. No string becomes HTML
    in the browser, so the sink is gone rather than guarded.

    Nothing a customer can type reached that markup: it is built from wc_price()
    over a float, and the only parts of it not fixed by the plugin are the currency
    symbol and price format, which come from the shop's own settings and from
    filters other extensions may attach. The exposure was to a misbehaving
    extension or a store's own configuration, not to a shopper — but the filtered
    and unfiltered paths were rendering the same markup, and only one of them was
    checked.

    .bogo-select__price-option[hidden] is stated in the stylesheet because a
    theme that gives spans a display of their own would otherwise defeat the
    browser's handling of hidden and show every variation's price at once.