Skip to content

fix(ENG-1961): defender plugin — payload floor, cue threshold, bump to 0.8.2 - #30

Merged
hiskudin merged 7 commits into
mainfrom
fix/ENG-1961-claude-plugin-only
Aug 20, 2026
Merged

fix(ENG-1961): defender plugin — payload floor, cue threshold, bump to 0.8.2#30
hiskudin merged 7 commits into
mainfrom
fix/ENG-1961-claude-plugin-only

Conversation

@hiskudin

@hiskudinhiskudin commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Four fixes to stackone-defender surfaced during Tier-2 false-positive root-causing.

1. PAYLOAD_SKIP_BELOW_BYTES: 500 → 32 (2628b8d) — the pre-daemon size gate silently dropped scans on any tool output under 500 bytes, including short overt injections that fit in ~120B. Verified via hook probe: same attack text flagged at 720B (with padding), silent at 114B. The gate exists to save an IPC round-trip on payloads defender itself would skip (per-string floor: 10 chars); 500B was a 50× overshoot of that rationale. New value covers the JSON wrapper (~20B) plus safety margin. Regression guard in tests/payload-skip-floor.test.mjs.

2. maxSentence display: 80 → 300 chars (17d24bc) — the offending chunk in a real Tier-2 FP was 331 chars long; the 80-char truncation hid what actually tripped the classifier. Diagnostic-only, no security impact.

3. @stackone/defender: ~0.7.0 → 0.8.2 (exact pin) (ef1d356) — picks up the shipped Tier-2 detect-and-gate work (decorative-run normalization, NFKC unicode fold, token-degeneracy guard). Was invisible to the shipped plugin because the previous stale lockfile held resolved version at 0.7.0. Also drops the tilde per the standing "exact pin, no ranges" convention, and adds the previously untracked package-lock.json so lockfile drift can't recur silently.

4. Suspicious cue threshold: 0.3 → 0.64 (d47054e) — plugin-side advisory tier fired below the library's own 0.5 medium-risk bar; on out-of-sample benchmarks the 0.3–0.5 band contributed disproportionate advisory noise per unit of true-positive signal. Aligns the advisory threshold with the block threshold: block or silent, no middle warn band. Matches the SKILL's silent-review posture.

Test plan

  • npm test — 13/13 (12 QA fixtures + ENG-1961 regression guard) on defender 0.8.2
  • Live-session testing in Claude Code — verify FP rate drops on real tool output, no regressions on realistic-injection fixtures
  • Peer hook-level probe battery + benchmark FPR sweep on the deployed build

Follow-ups (not in this PR)

  • Advisory else-branch in scan-tool-result.mjs is now effectively unreachable given the aligned threshold — left in place as an explicit no-op; can drop if the block-or-silent posture holds.

Summary by cubic

Reduces false positives and silent misses in the Defender plugin by scanning small tool outputs, aligning cues with blocking, and pinning dependencies.

  • Scan gate: lowers PAYLOAD_SKIP_BELOW_BYTES from 500 to 32 so sub-500B tool outputs are scanned; adds a constant-ceiling regression test and a below-floor hook smoke test; adds a long benign-text fixture to the QA suite; README reflects the ~32B floor.
  • Diagnostics: expands maxSentence display from 80 to 300 chars to aid triage.
  • Dependencies: pins @stackone/defender to 0.8.2 and adds package-lock.json; adds overrides for tar, protobufjs, sharp, adm-zip, and nanoid to clear audit issues.
  • Cues: aligns advisory with the block threshold (0.64) and removes the inert Suspicious branch; behavior is now block-or-silent.

Written for commit 1a9c61b. Summary will update on new commits.

Review in cubic

hiskudinand others added 4 commits August 19, 2026 17:10
The pre-daemon size gate silently dropped scans on any tool output
under 500 bytes — including short overt injections that fit in ~120B.
Verified via live hook probe on the installed 3.0.0: same attack text
flagged at 720B (with lorem-ipsum padding), silent at 114B.
The in-code comment justified the gate as an IPC-cost optimization for
payloads defender itself would skip (per-string floor: 10 chars). 500B
is a 50x overshoot of that rationale — the wrapper is ~20B, so 32B
covers it with a small safety margin.
Adds a regression test that greps the constant and fails if it climbs
back above 64. Cheap, fast, and pinned to the exact failure mode.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The offending chunk in a real Tier-2 FP was 331 chars long — the 80-char
truncation hid the box-drawing run that caused the false positive,
forcing the reviewer to bypass the hook and call the library directly
to see what actually tripped the classifier.
Diagnostic-only change; no security impact.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two changes rolled into one:
1. Version bump: ~0.7.0 → 0.8.2 (current npm latest). Picks up all
ENG-1084 Tier-2 detect-and-gate work — decorative-run normalization,
NFKC unicode fold, and the token-degeneracy guard — which was
invisible to the shipped plugin because the previous stale lockfile
held the resolved version at 0.7.0.
2. Exact pin (no tilde). StackOne convention across all repos is no
caret/tilde ranges in package.json — bare-version pins only. The
old ~0.7.0 is what let the stale-lockfile scenario happen in the
first place; a fresh install would have jumped ahead of the lockfile.
Also adds the previously untracked package-lock.json so lockfile drift
can't recur silently.
Verified: 13/13 tests pass on 0.8.2, including the ENG-1961 regression
guard and all 12 QA fixtures.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The plugin's "Suspicious" advisory tier fired on any Tier-2 score
above 0.3, well below the library's own 0.5 medium-risk bar. That
0.3–0.5 band contributed disproportionate advisory noise per unit of
true-positive signal — on out-of-sample blackXmask, dropping the
threshold from 0.5 to 0.3 buys +6pt TPR at the cost of +7pt FPR.
Lowering PAYLOAD_SKIP_BELOW_BYTES (2628b8d) surfaces more of this
noisy band by scanning smaller payloads.
Aligning the advisory threshold with the block threshold (0.64, from
defender-daemon.config.json's highRiskThreshold) collapses the tiers:
block or silent, no middle warn band. Cleaner semantic, and matches
the SKILL's silent-review posture. The advisory else-branch is now
effectively unreachable — any score >= 0.64 blocks first via the
library's blockHighRisk path. Left in place as an explicit no-op that
documents intent; a follow-up can drop it if this posture holds.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 19, 2026 16:27

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the stackone-defender security plugin to reduce false negatives/noise during Tier-2 false-positive root cause analysis by lowering the payload-size skip floor, updating the Tier-2 library dependency, and aligning cue thresholds and diagnostics with current Tier-2 behavior/config.

Changes:

  • Lowered the hook-side payload byte gate (PAYLOAD_SKIP_BELOW_BYTES) and added a regression test to prevent it from drifting upward.
  • Increased maxSentence diagnostic visibility and raised the “suspicious cue” threshold to align with configured Tier-2 thresholds.
  • Pinned @stackone/defender to 0.8.2 and added a lockfile to prevent silent resolution drift.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

FileDescription
plugins/security/stackone-defender/tests/payload-skip-floor.test.mjsAdds regression coverage to prevent raising the payload-skip floor back to overly large values.
plugins/security/stackone-defender/scripts/scan-tool-result.mjsLowers IPC skip floor, expands diagnostic maxSentence display, and adjusts suspicious-cue threshold.
plugins/security/stackone-defender/package.jsonPins @stackone/defender to 0.8.2.
plugins/security/stackone-defender/package-lock.jsonAdds lockfile to ensure deterministic dependency resolution.
Files not reviewed (1)
  • plugins/security/stackone-defender/package-lock.json: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment threadplugins/security/stackone-defender/package-lock.json
Comment threadplugins/security/stackone-defender/scripts/scan-tool-result.mjs Outdated
…ADME, dead branch, hook-level test
Four items from the PR-30 review round bundled together:
1. package.json overrides for tar/protobufjs/sharp/adm-zip/nanoid at the
Aikido-recommended patched versions. npm audit goes from 5H+1C to
zero. The lockfile was untracked on main, so these were pre-existing
in the resolved tree but only became visible once ENG-1961 committed
the lockfile.
2. README no longer says "500 bytes are skipped" in the 3 places it did.
3. Dead advisory else-branch removed from scan-tool-result.mjs.
Under the shipped multihead config defender's tier2Score is forced
binary, so any advisory tier off it is inert (verified against the
installed 0.8.2 dist). Note left pointing at tier2RawScore for
whoever wants a genuine sub-block advisory later.
4. Hook-level integration test payload-skip-hook.test.mjs asserts the
gate actually fires (below-floor payload = fast silent exit) and
isn't bypassed (above-floor payload = wall-clock exceeds fast-exit
budget). Complements the sibling grep-guard by catching structural
regressions the grep can't see.
Verification: 15/15 tests pass; npm audit clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment threadplugins/security/stackone-defender/tests/payload-skip-hook.test.mjs Outdated
Comment threadplugins/security/stackone-defender/package.json
…tion
The elapsedMs >= 300 threshold flakes when the defender daemon is
already warm at ~/.claude/defender.sock. Rewrote both tests to observe
status + stdout only: below-floor stays silent, above-floor benign
text must not produce a cue. The second assertion doubles as an FP
regression guard on 2000-char benign natural-language input.
Grep-based sibling payload-skip-floor.test.mjs remains the constant-
value regression guard.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment threadplugins/security/stackone-defender/tests/payload-skip-hook.test.mjs Outdated
…re instead
cubic correctly flagged that the second hook test's status===0/stdout==""
assertion passes vacuously whenever scanning doesn't happen (fail-open on
dep-install failure, daemon-down, timeout). Silent-exit stdio is
indistinguishable from "benign scanned as benign", so the FP-regression
claim was false.
Split the two coverage goals into places that can honor them:
- Below-floor gate: kept the single hook-spawn test (proves the hook
parses stdin and silent-exits cleanly on a tiny payload).
- FP regression on above-floor benign text: added
tests/fixtures/benign/long-natural-language.txt (2000-char natural
prose). Runs through the QA fixture suite in-process via
PromptDefense — no daemon dependency, deterministic pass/fail.
Verification: 15/15 tests pass (5 benign fixtures incl. the new one).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Requires human review: Lowers scan skip threshold, increases diagnostic truncation, pins defender dependency to 0.8.2, removes dead advisory branch. Requires human verification of dependency update and advisory removal.

Re-trigger cubic

@hiskudin
hiskudin merged commit c0b5d7e into mainAug 20, 2026
4 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@hiskudin