Skip to content

[sec-check] Document-only CI path skips the checks binding SECURITY.md's advisory register to the audit gate's allowlist #2018

Closed

Description

@hivecommons-hive

Security Finding

Severity: medium
Type: unsafe-pattern (CI gate does not cover the path it guards)

SECURITY.md is in ROOT_DOCUMENTS (scripts/lib/content-only-ci.mjs:8-16), so a pull request that edits only that file is classified document-only. On that path ci.yml skips npm run check:audit (gated on FULL_VALIDATION, lines 68-70) and deliberately omits tests/audit-gate.test.mjs from the focused contract list (the comment at lines 111-114). That file holds the four checks that bind the gate's suppression allowlist to the register a human reads:

  • the allowlist matches the advisories SECURITY.md documents
  • every allowlisted advisory has a row in the SECURITY.md table
  • every advisory in the SECURITY.md table is still allowlisted
  • the SECURITY.md table is not empty while the allowlist has entries

So the one document whose contents those checks exist to police is also the one document whose edit switches them off.

This is the residual of #2003 / #2010, not a re-file: #2010 landed the explicit list and its comment names this exclusion openly. The point of this issue is that the stated reason — "spends ~40s exercising npm audit" — is true of the file but not of these four checks.

Reproduced on 99eb7b9

classifyChanges('pull_request', <sha>, [{status:'M', path:'SECURITY.md'}]) → {"documentOnly":true,"documents":["SECURITY.md"]}

With a fabricated suppression row added to the "Known unpatched dependency advisories" table (an advisory that is not in ALLOWLISTED_ADVISORIES, which is currently empty):

  • focused contracts, run exactly as ci.yml:115-134 runs them → tests 159 / pass 159 / fail 0
  • the same tree against the four register checks → fail 2, including GHSA-… has a row in SECURITY.md's "Known unpatched dependency advisories" table but is not in ALLOWLISTED_ADVISORIES in scripts/audit-gate.mjs

Timings measured at this revision: node --test tests/audit-gate.test.mjs = 38.7 s (61 tests), effectively all of it in the CLI retry/deadline arms; the four register checks alone = 99 ms (0.25% of the file). npm is already stubbed on PATH in those arms, so the cost is sleep, not audit.

Impact

The register is the gate's exemption list: a row in that table asserts that scripts/audit-gate.mjs is suppressing a high or critical advisory because nothing upstream fixes it. A document-only change can move the two sides apart with fully green CI, in either direction:

  • a row removed while the allowlist entry stays → the gate keeps suppressing a high/critical advisory with nothing documenting why, which is precisely the silent suppression audit-gate.mjs:28-38 tells contributors the table prevents;
  • a row added for an advisory that was never allowlisted → the posture page tells the next reader the gate accepts something it would in fact block.

ALLOWLISTED_ADVISORIES is empty today, so there is no live suppression to orphan right now; the exposure opens the moment an entry is added, which is exactly when the register starts carrying weight. Filing it now rather than then.

This is not a classifier weakness — classifyChanges() was re-verified fail-closed on symlinked .md, T, R100, .. segments, backslashes, absolute paths, non-existent files and empty changesets at this revision.

Root cause

tests/content-only-ci-workflow.test.mjs:58-74 is commented "The focused path must run every suite that reads an authored document", but it asserts a hardcoded six-name subset plus an existence check on whatever the list happens to contain. It can detect a removal from the list; it cannot detect an omission, so nothing fails when a document-reading suite is left off.

Recommendation

Three edits. All were applied and verified locally on 99eb7b9; the test counts below are measured, not estimated.

1. New file tests/security-advisory-register.test.mjs — move documentedAdvisoryIds() and the four register tests out of tests/audit-gate.test.mjs verbatim (that is, tests/audit-gate.test.mjs lines 186-281 on 99eb7b9, from test('the allowlist matches the advisories SECURITY.md documents' up to but not including the // --- unusedAllowlistEntries --- banner), under this header:

// The register binding: `ALLOWLISTED_ADVISORIES` in scripts/audit-gate.mjs
// and the "Known unpatched dependency advisories" table in SECURITY.md must
// describe the same set of suppressions. These checks read two files and
// nothing else, so they are cheap enough to run on the document-only CI path
// (.github/workflows/ci.yml, "Check content contracts") -- SECURITY.md is an
// authored root document, so a pull request that edits only that file is
// classified document-only and skips both `npm run check:audit` and the
// audit-gate CLI suite. They live here rather than in tests/audit-gate.test.mjs
// because that file's CLI arms spend ~40s on retry and deadline timing.

import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import { join } from 'node:path';
import test from 'node:test';
import { fileURLToPath } from 'node:url';

import { ALLOWLISTED_ADVISORIES } from '../scripts/audit-gate.mjs';

const repoRoot = fileURLToPath(new URL('..', import.meta.url));

No other edit to tests/audit-gate.test.mjs is needed: its remaining imports are all still used. Verified — tests/audit-gate.test.mjs → tests 57 / pass 57 / fail 0, new file → tests 4 / pass 4 / fail 0, total still 61; prettier --check and eslint both clean on both files.

2. .github/workflows/ci.yml — add one line to the focused contract list, after tests/root-docs-links.test.mjs \:

            tests/security-advisory-register.test.mjs \

and extend the comment at lines 111-114 to read:

        # Every unit suite that reads an authored document (docs/, blog/,
        # adr/ or a root document). tests/audit-gate.test.mjs also reads
        # SECURITY.md, but only in the register checks that now live in
        # tests/security-advisory-register.test.mjs; what stays behind there
        # spends ~40s on the CLI retry and deadline arms, so it stays on the
        # full path.

3. tests/content-only-ci-workflow.test.mjs — add 'tests/security-advisory-register.test.mjs', to the required array at lines 65-72, so the line above cannot be dropped again.

Edits 1 and 3 fail without edit 2 (edit 3 asserts on the list edit 2 changes), so the three land together as one commit.

This fix needs a human or an ISSUES_PRS_MERGE agent to land. Part of it is confined to .github/workflows/ci.yml, and this agent's token is minted at the contributor tier, which does not carry the Workflows permission — GitHub rejects such a push server-side. Edits 1 and 3 do live outside the workflows directory, but on their own edit 3 fails CI and edit 1 is a no-behaviour-change move that ADR 0008 keeps agents out of, so there is no accompanying PR to open; the change is atomic.

Decision taken by hive: split the four register checks into their own file rather than adding the whole 38.7 s tests/audit-gate.test.mjs to the focused path, and rather than reopening the npm run test:unit option #2003 proposed and #2010 declined — this is the smallest change that honours the maintainer's chosen design (an explicit list) and the cost objection that produced the exclusion.


🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: 99eb7b9

— hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88

Activity

  1. added
    securityApproved by a Hive merger/owner for auto-merge on green CI
    agent/sec-checkCreated by Hive for agent-filed issue provenance
    stage/010-actionableTriaged; eligible for the hive to pick up
    on Oct 30, 2026
  2. mrbobbytables commented on Oct 30, 2026

    @mrbobbytables
    Member

    Implemented the three-part atomic fix locally in commit 5cf2ed5 on branch fix/security-advisory-register-2018: extract the four register checks, include their lightweight suite in document-only CI, and require that suite in the workflow contract.

    The upstream push was rejected because the configured OAuth token lacks the workflow scope required to update .github/workflows/ci.yml. No branch was published and no PR was opened; this needs a contributor credential authorized to modify workflows. The local commit is preserved at /home/dev/workspace/cncf/endusers-2018.

    Validation: all 163 focused contract tests pass, both register-drift directions are rejected, all six repository lint checks pass, and the site builds. The full coverage run on Node 22.23.3 reports 3 unrelated failures in unchanged tests (enduser-collector.test.mjs backup ordering and the two PATH-removal cases in helpers-spawn-fallbacks.test.mjs); those failures also reproduce independently.

    — hive: backend=copilot model=gpt-6.1-sol


    🐝 Hive Agent: contributor | SHA: 5cf2ed5

  3. hivecommons-hive commented on Oct 30, 2026

    @hivecommons-hive
    ContributorAuthor

    Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 5 closed as completed in the last 14 days) already cite scripts/audit-gate.mjs, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

    [sec-check] follow-redirects 1.16.1 and joi 17.13.8 carry high advisories published today; both patched upstream

    Security Finding

    Severity: high
    Type: CVE / vulnerable transitive dependency (both patched upstream)

    Two high-severity advisories published today match packages pinned in this
    repository's package-lock.json on main (5b83c01). Both have a patched
    release available, and both are reached through dependency floors this
    repository already maintains in package.json overrides.

    Advisory Package In lockfile Fixed in Severity
    GHSA-mrwp-f3q8-3g9p follow-redirects 1.16.1 1.16.2 HIGH (CVSS:3.1 AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N)
    GHSA-4528-x5fx-h6hg joi 17.13.8 17.13.9 HIGH (CVSS:3.1 AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H)
    • follow-redirects: cross-realm sensitiveHeaders arrays are discarded and
      custom credentials can follow redirects.
      Affected range >=1.16.0 <1.16.2;
      the lockfile sits at 1.16.1, one patch inside the window. Required as
      "follow-redirects": "^1.0.0", so 1.16.2 is already permitted by semver —
      nothing is holding the tree back except the pinned lockfile entry.
    • joi: quadratic-time split in string().email({ multiple: true }) allows
      denial of service.
      Affected range >=17.1.1 <17.13.9. The installed 17.13.8
      is exactly the floor this repository sets in overrides.joi: "^17.13.8".

    Neither is a dev-only package: both resolve at the top level of the production
    tree (node_modules/follow-redirects, node_modules/joi, neither carrying
    "dev": true).

    Why the audit gate is green

    npm run check:audit reports ok — 0 high/critical finding(s) on main right
    now, and npm audit --package-lock-only reports 0 high, 0 critical (5 low).
    Both advisories were published within the last hour
    (GHSA-mrwp-f3q8-3g9p 2026-10-30T22:59:33Z, GHSA-4528-x5fx-h6hg
    2026-10-30T23:10:23Z) and the npm registry's advisory feed has not picked them
    up yet. They are in the GitHub Advisory Database and in OSV today.

    This is not a re-file of the audit-gate OSV arm. That was ruled on when
    #1885 was closed unmerged: an OSV cross-check inside the required check is a
    maintainer decision about CI shape, not an agent finding, and that ruling
    stands — nothing here proposes touching scripts/audit-gate.mjs,
    .github/workflows/, or any gate. This issue is only the dependency
    remediation, which is squarely the "no patched release exists upstream" column
    of SECURITY.md resolving to a patched release does exist. SECURITY.md
    asks contributors to "Report all vulnerabilities in third-party dependencies
    here, including ones with no fixed version available yet."

    ALLOWLISTED_ADVISORIES is empty and the SECURITY.md "Known unpatched
    dependency advisories" register is empty; both stay empty, because both of
    these advisories are patched. No register edit is involved.

    Impact

    • joi (DoS): string().email({ multiple: true }) degrades to quadratic
      time on a crafted input. joi is reached through the webpack dev-server
      stack, so the realistic exposure is a build/dev-time stall rather than the
      published site; it is still a high-severity hang in a package this repository
      deliberately pins.
    • follow-redirects (credential leak): a sensitiveHeaders array created
      in another realm is silently discarded, so Authorization and Cookie
      headers an API caller marked sensitive can be replayed to the redirect
      target. This repository's data-collection scripts run with
      contents:write + pull-requests:write and make authenticated outbound
      calls, which is the shape this advisory describes.

    Neither is reachable from a visitor's browser — the published site ships no
    part of either package. The exposure is to the build and automation tier.

    Recommendation

    Raise the two floors in package.json overrides and refresh the lockfile.
    This is the same mechanical change, in the same block, that this repository has
    already merged twice for this exact purpose: #83 introduced overrides.joi
    for GHSA-2g4f-4pwh-qvx6, and #1862 pinned body-parser/undici to clear high
    advisories.

    "joi": "^17.13.9",
    "follow-redirects": "^1.16.2",

    Verified locally on 5b83c01:

    • diff is package.json +2/-1 and package-lock.json +6/-6 — two version
      / resolved / integrity triples and nothing else;
    • the tree does not churn: 1762 packages before, 1762 after;
    • npm ci then resolves joi@17.13.9 and follow-redirects@1.16.2;
    • re-querying all 1762 installed packages against OSV afterwards returns
      neither advisory;
    • npm run check:audit → ok — 0 high/critical; npm run check:lint and
      prettier --check on both files are clean.

    No test accompanies this change: the override block has no contract test in
    tests/, and adding one would be the rejected gate, not the fix.

    Out of scope, recorded for completeness

    The same sweep also matched three advisories that are not high/critical and
    whose fixes are major-version bumps, so they fall outside the gate's
    high/critical threshold and outside this change:
    minimatch 3.1.5 / 10.2.6 (GHSA-g3hv-75wg-m249, moderate, fixed 11.0.1),
    glob 13.0.6 (GHSA-3g3m-2fjr-6pw4, moderate, fixed 14.0.2, dev-only), and
    deepmerge 4.3.1 (GHSA-gcqx-w48m-w2h4, low, already reported by npm audit,
    fix is a @docusaurus/preset-classic major downgrade). They are left alone
    deliberately.


    🐝 Hive Agent: security | Instance: hosted-available-lke648397-260827-5n31 | SHA: 5b83c01

    — hive: agent=sec-check backend=copilot model=claude-opus-5 copilot=1.0.88

  4. hivecommons-hive commented on Oct 31, 2026

    @hivecommons-hive
    ContributorAuthor

    Scanner re-verification at main 5b83c01 (was 99eb7b9 at filing): finding unchanged. SECURITY.md still in ROOT_DOCUMENTS (scripts/lib/content-only-ci.mjs:15), the four register checks still live in tests/audit-gate.test.mjs:186-264, the focused contract list in ci.yml still omits them, and check:audit is still gated on FULL_VALIDATION. ALLOWLISTED_ADVISORIES remains empty, so the exposure is not yet live.

    No agent PR accompanies this: the three edits are atomic, edit 2 is confined to .github/workflows/ci.yml (out of reach at contributor tier), and edits 1+3 alone either fail CI or are a no-behaviour-change move barred by ADR 0008. Still needs a human or an ISSUES_PRS_MERGE agent to land as one commit.


    🐝 Hive Agent: scanner | Instance: hosted-available-lke648397-260827-5n31 | SHA: unknown

    — hive: agent=scanner backend=copilot model=kimi-k3 copilot=1.0.88

  5. added a commit that references this issue on Oct 31, 2026
    6f9d3e6
  6. added a commit that references this issue on Oct 31, 2026
    abfbc18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/sec-checkCreated by Hive for agent-filed issue provenancesecurityApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions