fix(learnings): fail closed when cross-project learning lacks trusted field#1746
Closed
jbetala7 wants to merge 2 commits into
Closed
fix(learnings): fail closed when cross-project learning lacks trusted field#1746jbetala7 wants to merge 2 commits into
jbetala7 wants to merge 2 commits into
Conversation
The --cross-project trust gate used a denylist (e.trusted === false), so rows with no trusted field (legacy rows written before the field existed in garrytan#988, hand-edited rows, or rows from other tools) were admitted because undefined === false is false. Switch to an allowlist (e.trusted !== true) to match the documented intent: cross-project learnings load only when explicitly trusted. Current-format rows are unaffected.
Closed
3 tasks
Contributor
Author
|
Closing as superseded: the fix for #1745 landed on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1745
Summary
trusted === truetrustedfieldRoot cause
gstack-learnings-search --cross-projectdocuments an allowlist ("cross-project learnings only loaded if trusted (user-stated)") to keep one project's AI-generated learnings from influencing reviews in another. The gate was a denylist:undefined === falseisfalse, so any cross-project row missing thetrustedfield was admitted. The field was introduced in security wave 3 (#988, 2026-04-13);gstack-learnings-searchshipped earlier (#622). Rows written before the field existed — plus hand-edited rows and rows from other tools — have notrustedkey and leak across projects today.Fix
Switch the gate to an allowlist (
e.trusted !== true). Current-format rows are unaffected:user-statedrows carrytrusted: trueand still load; everything else carriestrusted: falseand is still excluded. Only rows missing the field change behavior, and they now fail closed.Testing
bun test test/gstack-learnings-search.test.ts(6 pass; the new test fails on the pre-fix binary, confirming it guards the regression)bun test test/learnings.test.ts test/learnings-injection.test.ts(30 pass)observedrow with notrustedfield is no longer admitted; a foreignuser-stated/trusted: truerow still isgit diff --checkclean;git merge-treeclean againstorigin/main