feat(security): add per-plugin capability permissions and pre-execution enforcement#368
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6594c5459e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "capabilities": [ | ||
| "network" | ||
| ] |
There was a problem hiding this comment.
Adding capabilities changes the canonical metadata digest used by PluginManager.compute_plugin_digest, but the existing checksum value is left unchanged. Because _verify_plugin_integrity rejects any plugin with a mismatched checksum even when signature enforcement is off, every plugin whose metadata was edited here is skipped during load_plugins; I recomputed the digest for this file and it no longer matches the checked-in checksum, so the Nmap plugin will disappear from the API instead of gaining capability enforcement.
Useful? React with 👍 / 👎.
| plugin_signature_key: Optional[str] = None | ||
| enforce_plugin_signatures: bool = False | ||
| vault_key: Optional[str] = None | ||
| denied_capabilities: List[str] = [] |
There was a problem hiding this comment.
This new List[str] setting is documented in .env.example as SECUSCAN_DENIED_CAPABILITIES=exploit,credentials, but it is not included in the existing CSV parser validator used for other comma-separated list settings. In deployments that follow the documented example, pydantic-settings treats list environment values as JSON unless custom parsing is applied, so the server will either fail settings loading or never produce the intended ['exploit', 'credentials'] denied set; include this field in parse_csv_or_list (or use equivalent custom parsing) so the advertised policy knob works.
Useful? React with 👍 / 👎.
|
@utksh1 please review all my PRs, people have started to raise duplicate issues and are merging it but mine is not getting merged, please review into this matter, thank you |
utksh1
added
area:backend
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Requesting changes. Capability permissions are a good direction, but backend-tests are failing and the PR changes metadata across dozens of plugins in one shot. Please first get CI green, then narrow the PR to the core enforcement model plus a small representative set of plugin metadata and tests. Also document the migration/backward-compatibility behavior for plugins without capability declarations.
|
@utksh1 please review this pr, it is ready to merge, let me know if any improvements required otherwise it's good to review and get merged, thank you for assigning me this issue |
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Re-reviewed latest state. The capability model is useful, but the PR still changes too much metadata surface at once and needs a clearer backward-compatibility story for plugins without declarations. Please narrow to core enforcement plus representative plugin metadata, document migration behavior, and keep tests focused on allow/deny paths.
Plugins declare a capabilities list in metadata.json (network, filesystem, docker, credentials, intrusive, exploit). CapabilityEnforcer checks those declarations against the operator-configured SECUSCAN_DENIED_CAPABILITIES list before any command is built or subprocess is spawned. Legacy plugins without an explicit capabilities field fall back to an implied set derived from their safety level so enforcement works across the full plugin catalogue without a hard cut-over. - capabilities.py: Capability enum, CapabilityEnforcer, CapabilityDeniedError, validate_capability_list, effective_capabilities, build_enforcer_from_settings - models.py: optional capabilities field on PluginMetadata - config.py: denied_capabilities setting (SECUSCAN_DENIED_CAPABILITIES env var) - executor.py: capability check before command build; dedicated audit log event on denial; CapabilityDeniedError caught and persisted as task failure - plugins.py: validate capabilities at load time; expose in list_plugins response - All 60 plugin metadata.json files: explicit capabilities declarations - test_capabilities.py: 36 unit tests covering allow, deny, normalisation, legacy fallback, error metadata, and settings integration - .env.example: documented SECUSCAN_DENIED_CAPABILITIES with examples Closes utksh1#205
…e, document backward compat Revert all 60 plugin metadata.json changes: adding capabilities to every plugin file invalidated their checksums (capabilities is included in the digest computation), causing all plugins to fail to load and collapsing the test suite. Enforcement works for all existing plugins via the safety-level implied-capability fallback, so no metadata changes are needed for the feature to be active. Fix duplicate plugin display name: both waf-detection and waf_detector had the name "WAF Detector"; rename waf_detector to "WAF Detector (wafw00f)" and refresh its checksum. Add explicit backward-compatibility documentation to capabilities.py explaining the implied-set fallback and the checksum-refresh requirement for plugin authors who opt in to explicit capability declarations.
advikdivekar
force-pushed
the
security/per-plugin-capability-permissions
branch
from
1788481 to
1081c34
Compare
|
Re-reviewed after the latest push. Still blocked: please narrow this to core capability enforcement plus representative plugin metadata, document backward-compatibility behavior for plugins without declarations, and keep tests focused on allow/deny paths. |
…ctor Adds capabilities: ["network"] as a representative example showing how plugins should declare their required capabilities in metadata.json. Updates checksum to match the new canonical metadata.
|
@utksh1 please review this PR |
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Thanks for the latest capability-policy update. This still needs tightening before merge. The PR changes capability metadata for the full plugin catalog in the same branch as the enforcement engine, which makes individual capability assignments hard to audit. Please split or at least add a generated/checked inventory test that verifies every plugin has only recognized capabilities and that high-risk plugins are classified consistently. Also validate operator-denied capability tokens at settings/enforcer construction time; a typo in SECUSCAN_DENIED_CAPABILITIES should fail loudly instead of silently producing a policy that does not enforce what the operator intended.
Okay, I'll start working on it |
…inventory tests Two changes: 1. CapabilityEnforcer now raises ValueError at construction time when SECUSCAN_DENIED_CAPABILITIES contains an unrecognised token. A typo in the deny list previously silently produced an empty deny set, which would fail to enforce the intended policy. 2. New inventory test suite scans every plugin metadata.json and asserts: - All declared capability tokens are in ALL_CAPABILITIES - Exploit-level plugins that declare explicit capabilities include "exploit" - Intrusive-level plugins that declare explicit capabilities include "intrusive" - The capabilities field is a JSON array of lowercase strings CapabilityEnforcer construction-time validation is also fully covered.
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head. Still blocked: the current diff still carries broad plugin metadata/checksum churn while only one plugin ends up with an explicit capabilities example, so the security policy is difficult to audit and easy to regress. Please narrow this to the enforcement code plus a deliberate metadata migration, or add an inventory/regression test proving every shipped plugin has the expected effective capabilities and that denied capabilities block execution before command construction. Also avoid unrelated metadata rewrites such as icon encoding/order/checksum churn unless they are part of a scripted, documented migration.
…locking tests
metadata fix:
Revert waf_detector/metadata.json to upstream key order, icon encoding, and
name. The only intentional change from upstream is adding `capabilities:
["network"]` and updating the checksum. This eliminates all unrelated
encoding/ordering churn from the diff.
inventory tests (32 total):
- effective_capabilities implied-set correctness for all safety levels
(safe/intrusive/exploit/unknown) and the explicit-override behaviour
- All shipped plugins produce a non-empty effective_capabilities set
- CapabilityEnforcer.check() raises CapabilityDeniedError BEFORE any
fake build_command is called (execution-blocking regression)
- Exploit/intrusive plugins blocked when the matching cap is denied
- Safe plugin passes when only exploit is denied
- Plugin with explicit capabilities declaration blocked on denied token
- Empty deny-list never blocks any shipped plugin
- Denying "exploit" blocks EVERY exploit-level plugin in the plugins/ tree
|
@utksh1 please review this PR once |
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head after the inventory update and main merge. The PR now includes capability inventory coverage, denied-token validation, execution-blocking regression coverage, and green backend/frontend checks. Good to merge.
2 participants
What is the problem
SecuScan had no mechanism to restrict which capabilities individual plugins could exercise at runtime. An operator running in an air-gapped or restricted environment had no way to prevent plugins from making network calls, accessing secrets, or running exploits. The
safety.levelfield existed but was purely advisory with no enforcement boundary. Closes #205.What was changed
backend/secuscan/capabilities.pyCapabilityenum (network, filesystem, docker, credentials, intrusive, exploit),CapabilityEnforcer,CapabilityDeniedError,validate_capability_list,effective_capabilities,build_enforcer_from_settingsbackend/secuscan/models.pycapabilities: List[str]field toPluginMetadatabackend/secuscan/config.pydenied_capabilities: List[str]driven bySECUSCAN_DENIED_CAPABILITIESenv varbackend/secuscan/executor.pyCapabilityEnforcerat startup; callsenforcer.check()beforebuild_command(); catchesCapabilityDeniedErrorwith its own audit log event and persists the task as failedbackend/secuscan/plugins.pyplugins/waf_detector/metadata.json"capabilities": ["network"]testing/backend/unit/test_capabilities.py.env.exampleSECUSCAN_DENIED_CAPABILITIESwith examplesBackward compatibility — explicit documentation
Plugins that do not declare a
capabilitiesfield are not broken. An implied capability set is derived from their existingsafety.levelfield:safety.levelsafe["network"]intrusive["network", "intrusive"]exploit["network", "intrusive", "exploit"]This means:
SECUSCAN_DENIED_CAPABILITIES=exploit) and all exploit-level plugins are blocked, even without explicitcapabilitiesdeclarationscapabilitieslist for fine-grained visibility — only one representative example (waf_detector) is updated in this PR to show the patternWhy this approach
The check runs inside
execute_taskimmediately beforebuild_command(), so no subprocess is ever spawned for a denied plugin. The enforcer is constructed once at executor startup from settings, keeping the hot path allocation-free.How to test
SECUSCAN_DENIED_CAPABILITIES=exploit python -m secuscansqlmap,zap_scanner)failedwithtask_capability_deniedaudit log entrynmap(safe plugin) — runs normallypython -m pytest testing/backend/unit/test_capabilities.py -v— 36 tests passEdge cases covered
capabilitiesfield falls back to safety-level implied set (backward compatible)CapabilityDeniedErrorcarriesplugin_idanddenied_capabilitiesfor structured loggingLines changed
824 insertions, 122 deletions — 585 backend tests pass, 0 failures
Verification
Labels:
type:securitytype:featurelevel:advancedgssoc:approvedCloses #205
Please review and merge this under GSSoC 2026.