fix(core): fix TRUST_PARENT rule precedence in folder-trust resolution - #28701
fix(core): fix TRUST_PARENT rule precedence in folder-trust resolution#28701herdiyana256 wants to merge 2 commits into
Conversation
LoadedTrustedFolders.isPathTrusted() picks the winning trust rule for a given location by "longest match wins", intending the most specific configured rule to take precedence (confirmed by the existing "should handle isPathTrusted with longest match" test). For a TRUST_PARENT rule, the boundary it actually applies to is dirname(rulePath) -- one directory level shallower than the rule's own configured path -- but the specificity comparison used rulePath.length (the rule's own, deeper path) rather than the effective boundary's length. This overstates a TRUST_PARENT rule's specificity by the length of the child segment it explicitly strips away, letting it silently outrank a genuinely more specific sibling rule (e.g. an explicit DO_NOT_TRUST) that sits exactly at that parent boundary, whenever the TRUST_PARENT rule's own configured path string happens to be longer. "Trust parent folder" is one of only three choices in the standard folder-trust dialog shown the first time any folder is opened, so having a TRUST_PARENT rule recorded under a common parent directory is an ordinary, frequently-chosen outcome, not a contrived edge case. Fix: compare using the normalized effective path's length instead of the raw configured rulePath length, so specificity reflects the boundary a rule actually applies to.
|
📊 PR Size: size/M
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the folder-trust resolution mechanism where TRUST_PARENT rules were incorrectly prioritized due to an inaccurate specificity calculation. By using the normalized effective path length, the system now correctly identifies the intended boundary for trust rules, ensuring that more specific sibling rules are respected and preventing unintended trust grants. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes a specificity issue in the trust utility by comparing effective path lengths instead of raw rule path lengths, and adds a corresponding regression test. The reviewer identified a critical security concern where rules resolving to the same effective path length could lead to non-deterministic trust resolution, and provided a code suggestion to enforce a secure-by-default tie-breaker prioritizing DO_NOT_TRUST.
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
Comparing effective path lengths still leaves the case where two distinct rules resolve to the same boundary, e.g. DO_NOT_TRUST on `/a/b` and TRUST_PARENT on `/a/b/.gemini`. The strict `>` comparison then leaves the winner up to `Object.entries` iteration order, so an explicitly distrusted folder can resolve as trusted depending on config key order. Rank equal-length matches instead of ignoring them: DO_NOT_TRUST beats any grant, and an explicit TRUST_FOLDER on the boundary beats a TRUST_PARENT inherited from a rule one level deeper. Regression test asserts both key orders resolve to false; it fails without the tie-breaker.
|
@shrutip90 @DavidAPierce (and @emilyhedlund as the original author of |
|
Thank you for your response and for keeping this critical security-relevant
fix on track.
Regarding the broader context of managing this repository, I completely
appreciate your point that this is a "zero trust" endeavor. Maintaining
strict precedence rules and eliminating non-deterministic behavior is
exactly how we ensure the platform remains secure by default.
To speak to your note on code ownership, there is a fundamental difference
between authorship and stewardship. While authorship represents the initial
creation of a specific file or feature, stewardship is the ongoing
responsibility to protect, refine, and securely maintain the codebase over
time. Your pull request is a strong example of that stewardship in action,
addressing a critical logic gap to keep our trust resolution robust.
I will make sure our core team has visibility into this so we can prevent
the automated stale-bot from closing it before it receives a proper review.
With love and the utmost respect,
Russell Nordland
…On Thu, Aug 13, 2026 at 8:38 AM herdiyanitdev ***@***.***> wrote:
*herdiyana256* left a comment (google-gemini/gemini-cli#28701)
<#28701 (comment)>
@shrutip90 <https://github.com/shrutip90> @DavidAPierce
<https://github.com/DavidAPierce> (and @emilyhedlund
<https://github.com/emilyhedlund> as the original author of trust.ts),
flagging this one for review. It fixes a precedence bug in the
TRUST_PARENT rule during folder-trust resolution, where a parent folder's
trust setting can override a more specific child rule (CWE-863). The
stale-bot is set to auto-close it on 2026-08-20 for lacking a help wanted
issue, but this is a security-relevant correctness fix rather than a
feature request. Could one of you take a look before then? The change is
small and comes with regression tests. Thanks.
—
Reply to this email directly, view it on GitHub
<#28701?email_source=notifications&email_token=BPTYXQDKSBUG6H5TXYJN3RL5JXAF5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRYGEYTKNRWG4ZKM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#issuecomment-5281156672>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BPTYXQAOKFHRENOAZDR6TID5JXAF5AVCNFSNUABFKJSXA33TNF2G64TZHM4TMOBRHE3TEMJWHNEXG43VMU5TKMBXGIZDQNJWGM42C5QC>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
Reopening, the stale bot closed this after 14 days with no review. It fixes TRUST_PARENT rule precedence so a parent folder-trust setting can't override a more specific child rule (CWE-863). @DavidAPierce mind taking a look, or tagging help-wanted to keep the bot off it? Can rebase if needed. |
Summary
LoadedTrustedFolders.isPathTrusted()picks the winning trust rule for a given location by "longest match wins" -- intended to make the most specific configured rule take precedence (this is exactly what the existingshould handle isPathTrusted with longest matchtest verifies). For aTRUST_PARENTrule, the boundary it actually applies to isdirname(rulePath)-- one directory level shallower than the rule's own configured path -- but the specificity comparison usedrulePath.length(the rule's own, deeper path) instead of the effective boundary's length.This overstates a
TRUST_PARENTrule's specificity by the length of the child path segment it explicitly strips away viadirname(), letting it silently outrank a genuinely more specific sibling rule (e.g. an explicitDO_NOT_TRUST) that sits exactly at that parent boundary, whenever theTRUST_PARENTrule's own configured path string happens to be longer.Verified with the real
LoadedTrustedFoldersclass (not a reimplementation):"Trust parent folder" is one of only three choices in the standard folder-trust dialog shown the first time any folder is opened (
FolderTrustDialog.tsx), so ending up with aTRUST_PARENTrule recorded under a common parent directory is an ordinary, frequently-chosen outcome -- not a contrived configuration. Once such a rule exists anywhere under a shared parent, an unrelated sibling folder the user has explicitly markedDO_NOT_TRUST(or left unconfigured, if the rule's path length exceeds a siblingTRUST_FOLDER's -- less likely but structurally the same class of bug) can silently resolve as trusted, enabling project-sourced hooks and auto-connecting project-configured MCP servers against a folder the user never actually trusted.Fix
Compare using the normalized effective path's length instead of the raw configured
rulePathlength, so specificity reflects the boundary a rule actually applies to rather than the length of an arbitrary child segmentTRUST_PARENTstrips away. No behavior change forTRUST_FOLDER/DO_NOT_TRUSTrules, whereeffectivePath === rulePathalready.Test plan
trust.test.ts), confirming the explicitDO_NOT_TRUSTsibling now correctly wins, while unrelated siblings still correctly inherit theTRUST_PARENTgrant.npx vitest run --root packages/core packages/core/src/utils/trust.test.ts-- 13/13 passing (was 12/12 pre-fix, +1 new)npm run typecheck --workspace=@google/gemini-cli-corenpx eslinton both changed files -- clean