Repository navigation
fix(source-control): close five pre-existing secret and hold-path detection gaps (hardening deferred) #10943
Closed as not planned
Description
An independent security review of #10743's last round (c44bfb6) found no HIGH/CRITICAL and no regression. It found five gaps that predate that commit; #10743 does not widen them. Fix them after #10743 merges, because they touch the same files.
Findings
- MEDIUM: a quoted
inheritescapes both secret checks.babysit_risk.py_NON_JOB_SECRETand_SECRET_REFmatch\bsecrets\s*:\s*inherit\b.secrets: "inherit",secrets: 'inherit'and'secrets': inheritreturngated=False, and nosecret-exposure-reviewfires. Scenario: a PR addsuses: other/repo/.github/workflows/x.yml@shawithsecrets: "inherit"to a workflow outside the gate, and every repository secret reaches outside code after merge. Fix:\bsecrets['"]?\s*:\s*['"]?inherit\bin both, plus a test per quoted form. That GitHub parses a quotedinheritlike the bare word is UNVERIFIED; YAML treats them as the same string. - LOW: the comment strip cuts at
#inside quoted strings.line.split(" #", 1)[0]ingated_workflowand_code_lines.run: echo ' #' ${{ secrets.X }}is not gated. The patch-line check still holds it locally, but merge-holds.mjs reads only the generated GATE_WORKFLOWS table. Fix: strip only outside quotes and${{ }}, or scan YAML scalars. - LOW: a quoted
id-tokenkey escapes_MINTED_CREDENTIAL."id-token": writeis not gated. Fix:\b['"]?id-token['"]?\s*:\s*['"]?write\b. - LOW: a symlink named
.claudeor.claude-laneescapes the mjs and bash matchers. Python'sglob_matchholds the bare directory name, butmerge-holds.mjsmatchesHoldPathand thehold_patternsinpr-require-checks.ymldo not. A symlinkplugins/x/.claude -> ../../docs/payloadplus files underdocs/payload/passes the CI holds; only the local babysit lane (symlink = HIGH) catches it. Fix: match the directory name itself forX/**globs in mjs and bash, or hold any mode-120000 entry in both. Also make the matcher test assert all three are equal, not only that Python holds at least what Node holds. - LOW: a
pr-review-main: <lane>record from any workflow passes the app-slug check. A workflow withchecks: writeon a same-repo branch can post as the github-actions app. A forged clean record cannot lower a real CRITICAL (the max count wins, and a counted verdict from a successful main run is required), so the impact is a forged CRITICAL holding a PR. Optional hardening: bind the record to the run that uploaded the verdict.
Acceptance
- Each form above has a failing-then-passing test in
test_babysit_risk.py/test_merge_holds_sync.py. scripts/sync-hold-floor.py --checkstays green.
Refs: #10743
Activity
github-actions commentedon Oct 29, 2026
Proposed work class: work-class: scoped
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Known
Flags: none
Blocked by: none
Understood: five pre-existing detection gaps in the babysit risk checks and merge-hold matchers. They are a quoted
inheritand quotedid-tokenkey, comment stripping inside quoted strings, symlinked.claudedirectories, and an unboundpr-review-mainrecord. Each comes with a proposed fix, and acceptance is a failing-then-passing test per form withsync-hold-floor.py --checkstaying green. The issue says to start after #10743 merges. That is a PR, not an open issue, so I listed no blocker. No likely duplicates found among open issues.Automated triage, applied by the lanes App.
melodic-automation-lanes commentedon Oct 29, 2026
Not admitted to the issue lane: no trusted-human acceptance criteria: a listed human must put the confirmed
- [ ]criteria in the body or post them as their own comment, or, on an issue a listed human filed, intake's newest refinement comment must hold them unedited. Admission takes the issue once each of those is met, when intake triages it again (.github/actions/resolve-config/admission-rules.json).melodic-automation-lanes commentedon Oct 29, 2026
The latest triage verdict holds nothing: admission moved the issue to
status: confirmed; its comment says why.melodic-automation-lanes commentedon Oct 29, 2026
Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.
Intended outcome: Close the five detection gaps (quoted inherit, quote-aware comment strip, quoted id-token, symlinked .claude dirs, unbound pr-review-main record) with a failing-then-passing test each, after #10743 merges.
Dependencies
Related work
Open questions
RECOMMENDED: Hold the issue until #10743 merges, then re-run refinement against the merged base. Basis: .base/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py:908
RECOMMENDED: Match the bare directory name for X/** globs in both, so all three matchers agree with Python glob_match and the equality test can assert it. Basis: .base/.github/scripts/pr-merge/merge-holds.mjs:245
RECOMMENDED: Leave it out of this issue and file it separately, since it has no stated acceptance test. Basis: judgment
RECOMMENDED: Use a quote- and expression-aware strip, which is smaller than a YAML scan and stays dependency-free. Basis: .base/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py:955
RECOMMENDED: Treat it as inherit and gate it. Over-holding is safe, and YAML gives the same string. Basis: judgment
Sources read
To answer, post a comment that starts with this line:
Automated refinement by the lanes App. Answer with a comment whose first line is
**Decision recorded.**.kyle-sexton commentedon Oct 29, 2026
Two more gaps from the delta security review of #10743's 88bf5c9 (no HIGH/CRITICAL; neither is a loosening by that commit):
actions: write, because an edit inside existing scopes trips no patch hold. The dispatch logic is mostly outside the floor:.github/scripts/intake-reenter/reenter.mjs(itsWORKFLOWandrefconstants pick what theactions: writetoken dispatches),maintenance-reconcile-lost-runs/reconcile.mjs,post-merge-demotion-alert/{report,failing-checks}.mjs, andpr-unblock/{dispatch,fix-ci-dispatch,fix-docs-dispatch,dependency-dispatch}.mjs. A PR that re-points one of them leaves the workflow file unchanged and clears on a model verdict. Fix: havescripts/sync-hold-floor.pyderive the scripts each gated workflow runs (astest_the_floor_holds_what_its_actions_loadderives action closures) and hold themgate.actions: write/id-token: writemiss the text regex.actions: "\x77rite","\x61ctions": write,actions: !!str write, and block scalars (actions: >-thenwrite). Landing such a grant already holds through the YAML-parsed privilege-gain check, so this matters only for later edits. Fix: ingated_workflow, when PyYAML is available, decide from the parsed permissions (_workflow_privileges), keeping the regex as the fallback.kyle-sexton commentedon Oct 29, 2026
Gap found while building #10966:
lib/publish-gate.mjsis on no hold-floor row, yet.github/scripts/pr-review-main/write-verdict.mjs(a person-held gate path) imports it, so a PR can change how main-run verdicts are written with no hold. Not fixed in #10966: adding a person-held row adds an owner stop, which needs the owner's decision first. Options: hold itgate(one more owner stop on edits to it), or fold it into a lane-armed class once the #10966 attestation lands.[-]fix(source-control): close five pre-existing secret and hold-path detection gaps[/-][+]fix(source-control): close five pre-existing secret and hold-path detection gaps (hardening deferred)[/+]kyle-sexton commentedon Oct 30, 2026
Decision recorded. Hardening deferred, closed as not planned under owner standing direction §10: "Phase: maximum throughput until open issues and open PRs both reach 0. Hardening (re-adding holds, pins, reviews) is deferred to after that, as configuration edits."
Each of the five gaps is reachable only by our own actors. The secret-path ones depend on
secrets: inherit, which passes secrets only to workflows in the same organization or enterprise (https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows), so no outside actor can reach a token through them. §10 counts only a token or secret leak, account compromise or billing as a §3 boundary. Secret-leak scanning (gitleaks) stays on, as do both review lanes and ci-status (owner Q15). GitHub Support agreed with this classification.Reopen trigger: open issues = 0 and open PRs = 0. The hardening pass finds this with
is:issue is:closed reason:"not planned" in:title "hardening deferred".