Repository navigation
feat(extensions): least-privilege capability reconciliation (pm-fk84) - #326
Conversation
Detect capabilities an extension manifest declares but never exercises at
activation — the inverse of the existing missing-capability enforcement — so
package authors can trim manifests to least privilege.
Core (src/core/extensions/capability-usage.ts):
- EXTENSION_CAPABILITY_REGISTRATION_SURFACES: canonical capability->surface map,
`satisfies Record<ExtensionCapability,...>` makes it exhaustive at compile time
so a new capability without a surface fails the build (no perpetual false
"unused" positive).
- collectUsedExtensionCapabilities / reconcileExtensionCapabilityUsage: pure
functions over the activation result; declared is filtered to KNOWN
capabilities (unknowns stay the separate diagnostic); keyed by a JSON
[layer,name] tuple to keep same-named extensions on different layers distinct.
Doctor (extension/doctor.ts + extension.ts):
- classifyUnusedCapabilityWarnings emits advisory
extension_capability_unused:<layer>:<name>:<capability> plus a least-privilege
remediation hint. Advisory only — doctor `ok` stays true and it is
deliberately NOT wired into pm health (any non-telemetry/history extension
warning would block the health gate and the daily release).
SDK (sdk/testing.ts + sdk/index.ts + core/extensions/index.ts):
- assertExtensionCapabilityUsage(activation, { declared, extensionName?,
allowUnused? }) — package-test counterpart that fails on over-declaration.
- Exported the reconciliation helpers + surfaces map from the SDK barrel.
Docs: EXTENSIONS.md (advisory warning) and SDK.md (helper + exports).
100% coverage (4807 tests); typecheck, quality:static (docstrings),
docs-skills, changelog, and secret-scan all green. Repo's own 10 first-party
packages + builtins are already least-privilege (0 warnings).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CHP2yQxWtXG8ZfYvRSoW33
Reviewer's GuideImplements least-privilege capability reconciliation for extensions by tracking which capabilities are actually exercised at activation, surfacing declared-but-unused capabilities as advisory doctor warnings and as a failing SDK test helper, while exposing the core reconciliation logic and updating docs and changelog. Sequence diagram for extension doctor least-privilege capability warningssequenceDiagram
actor PackageAuthor
participant ExtensionDoctor as extension_doctor_command
participant Loader as loadExtensions
participant Activator as activateExtensions
participant UnusedClassifier as classifyUnusedCapabilityWarnings
participant Reconciler as reconcileExtensionCapabilityUsage
PackageAuthor->>ExtensionDoctor: runExtension --doctor
ExtensionDoctor->>Loader: loadExtensions
Loader-->>ExtensionDoctor: loadResult
ExtensionDoctor->>Activator: activateExtensions
Activator-->>ExtensionDoctor: activationResult
ExtensionDoctor->>UnusedClassifier: classifyUnusedCapabilityWarnings(loadResult, activationResult)
UnusedClassifier->>Reconciler: reconcileExtensionCapabilityUsage(loadResult.loaded, activationResult)
Reconciler-->>UnusedClassifier: ExtensionCapabilityUsageReconciliation[]
UnusedClassifier-->>ExtensionDoctor: ["extension_capability_unused:…"]
ExtensionDoctor->>ExtensionDoctor: buildExtensionTriageSummary(warnings)
ExtensionDoctor-->>PackageAuthor: advisory extension_capability_unused warnings
Sequence diagram for SDK assertExtensionCapabilityUsage helpersequenceDiagram
actor PackageTest as Package_test
participant Assertion as assertExtensionCapabilityUsage
participant CapabilityCheck as isKnownExtensionCapability
participant UsageCollector as collectUsedExtensionCapabilities
PackageTest->>Assertion: assertExtensionCapabilityUsage(activation, expectation)
loop for each expectation.declared
Assertion->>CapabilityCheck: isKnownExtensionCapability(normalized)
CapabilityCheck-->>Assertion: boolean
Assertion->>Assertion: add normalized to declaredSet
end
Assertion->>UsageCollector: collectUsedExtensionCapabilities(activation, { extensionName })
UsageCollector-->>Assertion: used capabilities
Assertion->>Assertion: compute unused = declared - used - allowUnused
alt unused is not empty
Assertion-->>PackageTest: throw Error
else unused is empty
Assertion-->>PackageTest: return { declared, used, unused }
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai full review |
|
@codex review |
|
/gemini review |
✅ Action performedFull review finished. |
|
Warning Review limit reached
More reviews will be available in 42 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds least-privilege capability reconciliation to the ChangesLeast-privilege extension capability reconciliation
Sequence Diagram(s)sequenceDiagram
participant Author as Extension Author
participant DoctorAction as pm package doctor
participant Classifier as classifyUnusedCapabilityWarnings
participant Reconciler as reconcileExtensionCapabilityUsage
participant Triage as buildExtensionTriageSummary
rect rgba(70, 130, 180, 0.5)
note over DoctorAction,Reconciler: Doctor CLI flow
DoctorAction->>Classifier: classifyUnusedCapabilityWarnings(loadResult, activationResult)
Classifier->>Reconciler: reconcileExtensionCapabilityUsage(loaded, activation)
Reconciler-->>Classifier: [{declared, used, unused}] per extension
Classifier-->>DoctorAction: extension_capability_unused:layer:name:cap tokens
DoctorAction->>Triage: warnings[]
Triage-->>DoctorAction: warn status + least-privilege remediation text
end
rect rgba(60, 179, 113, 0.5)
note over Author,Reconciler: SDK test-time assertion
Author->>assertExtensionCapabilityUsage: activation, {declared, extensionName, allowUnused}
assertExtensionCapabilityUsage->>Reconciler: collectUsedExtensionCapabilities(activation, filter)
Reconciler-->>assertExtensionCapabilityUsage: used[]
assertExtensionCapabilityUsage-->>Author: {declared, used, unused} or throws on over-declaration
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
To use Codex here, create a Codex account and connect to github. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces least-privilege capability reconciliation for extensions, detecting capabilities declared in manifests but never exercised during activation. It adds advisory doctor warnings (extension_capability_unused) and an SDK testing helper (assertExtensionCapabilityUsage) to catch over-declaration. Feedback on the changes suggests optimizing the lookup key serialization in capability-usage.ts by replacing JSON serialization with a simpler delimiter-based string to reduce performance overhead, and validating the allowUnused capabilities in the SDK helper to prevent silent typos.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function usageKey(layer: ExtensionLayer, name: string): string { | ||
| return JSON.stringify([layer, normalizeExtensionName(name)]); | ||
| } | ||
|
|
||
| function usageKeyName(key: string): string { | ||
| return (JSON.parse(key) as [ExtensionLayer, string])[1]; | ||
| } |
There was a problem hiding this comment.
Using JSON.stringify and JSON.parse to serialize and deserialize the lookup key introduces unnecessary performance overhead. Since layer is strictly typed as ExtensionLayer ("global" | "project"), it can never contain a delimiter like |. We can use a simple delimiter-based string serialization which is 100% collision-proof, cleaner, and significantly faster.
| function usageKey(layer: ExtensionLayer, name: string): string { | |
| return JSON.stringify([layer, normalizeExtensionName(name)]); | |
| } | |
| function usageKeyName(key: string): string { | |
| return (JSON.parse(key) as [ExtensionLayer, string])[1]; | |
| } | |
| function usageKey(layer: ExtensionLayer, name: string): string { | |
| return layer + "|" + normalizeExtensionName(name); | |
| } | |
| function usageKeyName(key: string): string { | |
| return key.slice(key.indexOf("|") + 1); | |
| } |
| const declared = [...declaredSet].sort((left, right) => left.localeCompare(right)); | ||
| const used = collectUsedExtensionCapabilities(activation, { extensionName: expectation.extensionName }); | ||
| const usedSet = new Set(used); | ||
| const allowUnused = new Set((expectation.allowUnused ?? []).map((capability) => capability.trim().toLowerCase())); |
There was a problem hiding this comment.
While expectation.declared is validated to ensure all entries are known extension capabilities, expectation.allowUnused is not validated. If a user makes a typo in allowUnused (e.g., "schem" instead of "schema"), it will be silently ignored, potentially leading to confusing test failures. We should validate expectation.allowUnused in the same way.
| const allowUnused = new Set((expectation.allowUnused ?? []).map((capability) => capability.trim().toLowerCase())); | |
| const allowUnused = new Set<ExtensionCapability>(); | |
| if (expectation.allowUnused) { | |
| for (const capability of expectation.allowUnused) { | |
| const normalized = capability.trim().toLowerCase(); | |
| if (!isKnownExtensionCapability(normalized)) { | |
| throw new Error( | |
| "Expected allowed unused capability \"" + capability + "\" to be a known extension capability. " + | |
| "Use canonical capability names (see manifest.capabilities)." | |
| ); | |
| } | |
| allowUnused.add(normalized); | |
| } | |
| } |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The capability normalization and known-capability filtering logic is duplicated between
normalizeDeclaredCapabilitiesincapability-usage.tsandassertExtensionCapabilityUsage; consider centralizing this in a shared helper to keep behavior and error messaging consistent over time. - In
assertExtensionCapabilityUsage, entries inallowUnusedare normalized but not validated viaisKnownExtensionCapability, which means a typo can silently bypass the least-privilege guard; it may be safer to apply the same known-capability validation used fordeclared. - The reconciliation logic currently keys usage by
layer+ normalizednamebut only exposes the originalnameinExtensionCapabilityUsageReconciliation; if names can differ by case or whitespace between manifest and activation, consider returning both original and normalized names or enforcing a single canonical form earlier to avoid subtle mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The capability normalization and known-capability filtering logic is duplicated between `normalizeDeclaredCapabilities` in `capability-usage.ts` and `assertExtensionCapabilityUsage`; consider centralizing this in a shared helper to keep behavior and error messaging consistent over time.
- In `assertExtensionCapabilityUsage`, entries in `allowUnused` are normalized but not validated via `isKnownExtensionCapability`, which means a typo can silently bypass the least-privilege guard; it may be safer to apply the same known-capability validation used for `declared`.
- The reconciliation logic currently keys usage by `layer` + normalized `name` but only exposes the original `name` in `ExtensionCapabilityUsageReconciliation`; if names can differ by case or whitespace between manifest and activation, consider returning both original and normalized names or enforcing a single canonical form earlier to avoid subtle mismatches.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request implements least-privilege capability reconciliation for extensions, flagging declared-but-unused capabilities as advisory warnings in the package doctor and introducing the assertExtensionCapabilityUsage SDK helper for testing. The core logic is added in a new capability-usage.ts module, with corresponding CLI integration, documentation updates, and unit tests. Feedback on the implementation suggests optimizing the lookup key serialization in capability-usage.ts by replacing JSON.stringify and JSON.parse with a delimited string to reduce CPU and memory overhead in hot paths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function usageKey(layer: ExtensionLayer, name: string): string { | ||
| return JSON.stringify([layer, normalizeExtensionName(name)]); | ||
| } | ||
|
|
||
| function usageKeyName(key: string): string { | ||
| return (JSON.parse(key) as [ExtensionLayer, string])[1]; | ||
| } |
There was a problem hiding this comment.
Using JSON.stringify and JSON.parse to serialize and deserialize the lookup key in a hot path (such as attributeCapabilityUsage and collectUsedExtensionCapabilities which iterate over all registrations) introduces unnecessary CPU and memory overhead.
Since layer is strictly "global" | "project", we can use a safe delimiter like \u0000 (NUL character escape sequence) to separate the layer and the normalized name. This avoids embedding a literal NUL byte in the source file (preserving grep compatibility) while being significantly faster than JSON serialization.
| function usageKey(layer: ExtensionLayer, name: string): string { | |
| return JSON.stringify([layer, normalizeExtensionName(name)]); | |
| } | |
| function usageKeyName(key: string): string { | |
| return (JSON.parse(key) as [ExtensionLayer, string])[1]; | |
| } | |
| function usageKey(layer: ExtensionLayer, name: string): string { | |
| return `${layer}\\u0000${normalizeExtensionName(name)}`; | |
| } | |
| function usageKeyName(key: string): string { | |
| return key.split("\\u0000")[1]; | |
| } |
…conciliation
- Replace the JSON-tuple usage key with `${layer}:${name}`. `layer` is a closed
enum that never contains a colon, so the first colon always separates layer
from the (possibly colon-bearing) name — collision-proof without
serialization overhead (Gemini).
- Validate `allowUnused` entries in assertExtensionCapabilityUsage so a typo
cannot silently bypass the least-privilege guard (Gemini + Sourcery).
- Extract the shared `normalizeKnownExtensionCapability` token helper into
extension-capability-aliases.js and use it from both the core reconciliation
and the SDK assertion, removing duplicated trim/lower/known-filter logic
(Sourcery).
100% coverage maintained (4807 tests); typecheck + quality:static green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CHP2yQxWtXG8ZfYvRSoW33
|
Thanks for the reviews. Addressed in 58fadb2:
100% coverage maintained (4807 tests); typecheck + quality:static green. @coderabbitai full review |
|
To use Codex here, create a Codex account and connect to github. |
|
Triggering a fresh full review now. ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements least-privilege capability reconciliation for extension packages. It introduces core logic to detect declared-but-unused capabilities at activation, surfaces these as advisory warnings in the package doctor tool, and provides a new SDK test helper assertExtensionCapabilityUsage for package authors to catch over-declaration during testing. The changes are well-documented, thoroughly tested, and integrate cleanly with the existing codebase. No review comments were provided, and I have no additional feedback.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
Closes the least-privilege loop for the extension/package platform. The loader already blocks an extension that registers a surface whose capability is undeclared (
extension_capability_missing). This PR adds the inverse, advisory direction: detect capabilities a manifest declares but never exercises at activation, so package authors can trim manifests to the minimum grant.Tracks pm-fk84 under epic pm-ugqx (extension & package platform maturity). Complements pm-mwwp (missing-capability enforcement) and pm-bav0 (scaffold least-privilege metadata).
What changed
Core —
src/core/extensions/capability-usage.ts(new)EXTENSION_CAPABILITY_REGISTRATION_SURFACES: canonical capability→surface map.satisfies Record<ExtensionCapability, …>makes it exhaustive at compile time — adding a capability without a registration surface fails the build, which prevents a capability with no surface from being perpetually flagged "unused".collectUsedExtensionCapabilities(activation, { extensionName? })andreconcileExtensionCapabilityUsage(loaded, activation): pure functions over the activation result. Declared capabilities are filtered to known ones (unknowns remain the separateextension_capability_unknowndiagnostic); entries are keyed by a JSON[layer, name]tuple so same-named extensions on different layers stay distinct.Doctor —
extension/doctor.ts+extension.tsclassifyUnusedCapabilityWarningsemits advisoryextension_capability_unused:<layer>:<name>:<capability>plus a least-privilege remediation hint in the triage summary.okstaystrue(status: "warn"). It is deliberately not wired intopm health—isAdvisoryHealthWarningonly whiteliststelemetry_*/history_*, so any other extension warning would block the health gate and the daily auto-release. Verifiedpm healthis unaffected.SDK —
sdk/testing.ts+sdk/index.ts+core/extensions/index.tsassertExtensionCapabilityUsage(activation, { declared, extensionName?, allowUnused? }): the package-test counterpart of the advisory warning. Pass the same capabilities asmanifest.capabilities; it throws on over-declaration. Rejects unknown declared capabilities. Barrel-re-exported with a barrel-equality lock.collectUsedExtensionCapabilities,reconcileExtensionCapabilityUsage, andEXTENSION_CAPABILITY_REGISTRATION_SURFACESfrom the SDK barrel.Docs —
EXTENSIONS.md(advisory warning + workflow) andSDK.md(helper + exports).Verification
typecheck,quality:static(docstring coverage),quality:docs-skills,changelog:pm:check(no drift), andsecurity:scanall green.["commands","schema","search"]that only registers a command is flagged forschema+search; trimming to["commands"]clears it.🤖 Generated with Claude Code
https://claude.ai/code/session_01CHP2yQxWtXG8ZfYvRSoW33
Summary by Sourcery
Introduce least-privilege capability reconciliation for extensions by comparing declared capabilities with those actually exercised at activation, surfacing over-declared permissions via doctor warnings, SDK helpers, and documentation updates.
New Features:
Enhancements:
pm package doctorto emit advisoryextension_capability_unusedwarnings with least-privilege remediation guidance while keeping health checks non-blocking.Documentation:
EXTENSIONS.mdandSDK.md.