Skip to content

feat: analyze bundled permission grants - #429

Open
chrisknvidia wants to merge 53 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-399-permission-surface
Open

feat: analyze bundled permission grants#429
chrisknvidia wants to merge 53 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-399-permission-surface

Conversation

@chrisknvidia

@chrisknvidia chrisknvidia commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #399

Depends on #404 and contains that branch as an explicit merge parent. GitHub cannot select a fork-only head branch as this upstream PR base, so the displayed base remains main; after #404 merges, this PR displayed diff collapses to the BH3-only delta below.

Scope

This draft is the BH3-only follow-up for the project-settings portion of #399:

  • inspect exact .claude/settings.json and .claude/settings.local.json surfaces
  • classify the issue-listed broad permissions.allow, root/home additionalDirectories, and permissions.defaultMode declarations
  • aggregate one deterministic, sanitized BH3 finding per physical settings document
  • apply the blocking score floor only to conditional CRITICAL grants

BH1/BH2 and shared analyzer wiring come from #404. The reviewable stacked delta is six files: 557 additions, 17 deletions.

Implementation

  • bounded declarative classifiers for whole-tool, root/home, sensitive-path, directory, and mode grants
  • defaultMode: auto is retained as LOW/ignored_by_surface on project/local settings, matching current documented behavior
  • effective project and local permission findings use the source-neutral requires_settings_activation qualifier
  • malformed modeled settings remain visible through PARTIAL ledger coverage and cannot make disableAllHooks trustworthy
  • root/home lexical normalization covers the approved closed forms while preserving exact-two-slash UNC exclusions
  • BH3 evidence contains only bounded kinds/counts and activation qualifiers; no permission engine, glob execution, or full settings-precedence simulation

Verification

  • focused analyzer, acceptance, registry, report, and CLI suite: 320 passed
  • full suite on exact stacked head bfef00f: 3,071 passed, 13 skipped, 38 deselected, 4 expected xfails
  • lint, formatting, C901 complexity, targeted mypy, lockfile, and stacked diff checks: clean
  • real mixed CLI scan: effective disableAllHooks suppressed ordinary BH1/BH2 plugin-hook findings while Bash(*) still emitted only BH3, remained complete, scored 51/HIGH, recommended DO_NOT_INSTALL, and exited 1
  • real local-settings CLI scan emitted BH3 with requires_settings_activation
  • malformed same-file modeled settings retained the valid hook finding and exposed the default versus --fail-on-incomplete exit contract
  • Docker smoke: image build, version/git checks, local fixture scan, and public GitHub URL scan passed
  • integration regression comparison with clean origin/main: both produced 30 passed, 2 skipped, 3 failed; the same two live-Codex model-resolution failures and graph/LLM telemetry expectation reproduce on main
  • independent post-fix review found no actionable defects after exercising global-disable/BH3 interaction, local precedence, malformed siblings, root/home forms, and ignored auto behavior

Deliberate limits

The analyzer does not calculate ask/deny precedence, execute permission globs, emulate tool aliases, resolve marketplace policy, or claim final runtime activation. Live authenticated Claude/IDE/Desktop permission dispatch was not run because the local Claude environment is not authenticated.

This PR remains draft for maintainer review.

chrisknvidia and others added 5 commits August 20, 2026 17:29
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
…ue-399-hook-surface

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>

@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]

This draft is not yet code-reviewable for its stated BH3 permission-grant scope. PR #429 and dependency PR #404 currently point to the identical head commit (8cf3376), so the displayed 18k-line diff is entirely inherited BH1/BH2 work and there are no BH3-specific production or test changes to evaluate.

Before requesting re-review, please:

  1. Fast-forward/rebase this branch onto the final reviewed #404 head. #404 currently conflicts with main, so resolve that dependency conflict first and propagate the resolved head here.
  2. Add the BH3 implementation and its focused positive, negative, boundary, archive, ledger, scoring, output-format, and baseline regressions described in this PR.
  3. Isolate the stacked diff while #404 remains open—preferably by temporarily targeting #404's branch as this PR's base—so reviewers see only BH3 changes rather than re-reviewing the entire dependency.
  4. Align the documented Claude Code semantics snapshot: this PR description names 2.1.241, while the inherited README/design currently state 2.1.238.

No inline findings are attached because this head contains no #429-specific code. Please keep the PR in draft and ping for re-review after the BH3 delta is present and the dependency/base conflicts are resolved.

@rng1995
rng1995 marked this pull request as ready for review August 24, 2026 18:33

@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.

Reviewed this head against #404. It is the exact same commit (8cf33768) and contains no permission-grant/BH3 implementation to review; the PR description also says production implementation has not started and depends on #404. I left the hook-flow findings on #404 to avoid duplicate inline threads. Please add the #429-specific implementation (and return this to draft until then), then re-request review.

@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.

Requesting changes because this PR is marked ready for review but has no #429-specific implementation: its head is byte-for-byte identical to #404, and the PR description says production implementation has not started. Please return it to draft, add the permission-grant/BH3 delta after its dependency lands, and then re-request review. The hook-flow implementation findings remain on #404 to avoid duplicate threads.

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia marked this pull request as draft August 24, 2026 19:08
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia

Copy link
Copy Markdown
Contributor Author

Implementation checkpoint: the BH3 delta is now present and pushed through 6dce677. The reviewable stacked range is 5f66e0e..6dce677; #404 remains the dependency. All hosted checks are green, the broad non-provider suite passed, issue-specific and archive/output E2E passed, and practical Docker plus one live Claude Code 2.1.241 trust probe were completed. The one practical integration failure was reproduced unchanged on fresh origin/main and is unrelated to this branch.

The existing changes-requested review targets old commit 8cf3376, when this branch had no BH3 changes. I am leaving that review untouched for the reviewer and keeping this PR in draft as requested.

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@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: new feature
  • Confidence: medium-low
  • Status read: Draft, changes requested, merge blocked
  • 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: Permission-grant/BH3 surface depends on feat: analyze bundled hook execution surfaces #404 and inherits unresolved hook-flow risk. Large new BH3 logic should not be promoted independently.
  • Recommended action: Do not promote as ready; de-risk feat: analyze bundled hook execution surfaces #404 first, then request fresh review on BH3-specific changes.

@rng1995
rng1995 marked this pull request as ready for review August 26, 2026 19:02

@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]

Re-review: the BH3 implementation is now present as an isolated permission-grant analyzer delta, the current-main conflict is resolved with the non-brittle README wording preserved, focused validation passed, and all required checks are green. Approved.

@rng1995
rng1995 enabled auto-merge (squash) August 26, 2026 20:08
@chrisknvidia
chrisknvidia marked this pull request as draft August 26, 2026 21:21
auto-merge was automatically disabled August 26, 2026 21:21

Pull request was converted to draft

Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia force-pushed the feat/christopherk/issue-399-permission-surface branch from bf5fd15 to d2fcb8b Compare August 26, 2026 22:17
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
@chrisknvidia
chrisknvidia marked this pull request as ready for review August 27, 2026 01:22
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.

3 participants