Skip to content

fix(pe3): require a credential-store access shape, not a bare noun - #424

Open
rodboev wants to merge 10 commits into
NVIDIA:mainfrom
rodboev:pr/pe3-keyring-396
Open

fix(pe3): require a credential-store access shape, not a bare noun#424
rodboev wants to merge 10 commits into
NVIDIA:mainfrom
rodboev:pr/pe3-keyring-396

Conversation

@rodboev

@rodboev rodboev commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

PE3 now requires a credential-store operation tied to the matched keyring or keychain noun in Markdown and text prose. Bare descriptive nouns remain clean; actionable reads, writes, mutations, API or CLI commands, fenced content, and code or configuration remain detected.

Root cause

The vocabulary pattern flagged credential-store nouns without requiring an access shape. A nearby-word qualifier could also let benign reading text suppress a later operation.

Diff Notes

  • Bound operation matching to the exact noun and its line or clause, including post-noun operations.
  • Kept access-policy, API-documentation, CLI-reference, lookup, query, and benign-reading prose clean while preserving actionable operations.
  • Centralized shared fence lexemes and logical line-break handling, while keeping analyzer and runner fence walkers separate.

Scope

Limited to PE3 credential-store qualification, shared fence and line handling, and focused regressions. Other PE rules and finding, report, severity, score, recommendation, suppression, and SARIF contracts are unchanged.

Closes #396

@rodboev
rodboev force-pushed the pr/pe3-keyring-396 branch from c74ba56 to cbcf954 Compare August 23, 2026 17:36
@yashrajp22
yashrajp22 self-requested a review August 24, 2026 10:08

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]

Requesting changes for a remaining PE3 false-positive path. The new nearby-word test treats descriptive access terminology as an access action, so ordinary prose can still become a high-severity credential-access finding. Please tighten the relationship between the store noun and an actual read/retrieve/mutation instruction and add a descriptive keyring access policy-style regression.

Comment thread src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py Outdated
@rng1995

rng1995 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

[SkillSpector Maintainer Ping]

@rodboev, please address the outstanding PE3 review feedback on this PR. Descriptive prose such as This section documents the keyring access policy. must not become a HIGH credential-access finding merely because access appears near keyring. Please tighten the action relationship, add the requested descriptive-prose regression, resolve the review thread, and reply here when the branch is ready for re-review.

The branch is currently behind main. GitHub reports it as mergeable with no conflicts, but please update it from main, resolve any conflicts that surface during that update, and rerun the required checks.

Comment thread src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py Outdated
Comment thread src/skillspector/nodes/analyzers/static_runner.py Outdated
Comment thread src/skillspector/nodes/analyzers/static_runner.py Outdated
Comment thread src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py Outdated
rodboev added 10 commits August 24, 2026 16:07
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
@rodboev
rodboev force-pushed the pr/pe3-keyring-396 branch from 5e7347d to b0b3752 Compare August 25, 2026 00:00
@mohgupta-ship-it

mohgupta-ship-it commented Aug 25, 2026

Copy link
Copy Markdown
Member

Powered by Codex: PR council review result.

This is a triage signal, not a maintainer approval.

  • Rating: bugfix
  • Confidence: medium-high
  • Status read: Changes requested, green checks
  • Review method: fresh GitHub metadata/body/files/reviews/checks plus selected diffs; council lenses were spec fit, dead-code/reachability, YAGNI/scope, design/coupling, and code standards/tests.
  • Council assessment: Narrows PE3 credential-store detection from bare noun matches toward access-shape matches. Sampled current head appears to address the prior false-positive class, but review remains unresolved.
  • Recommended action: Request focused re-review before merge.

Sign up for free to 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.

PE3 fires HIGH on credential vocabulary in prose: a SKILL.md containing only the word keyring is Privilege Escalation

4 participants