feat(injectors): emit finding-compatible outputs for shodan and aws (#396) - #421
feat(injectors): emit finding-compatible outputs for shodan and aws (#396)#421Samuel Hassine (SamuelHassine) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Shodan and AWS injectors to emit finding-compatible structured outputs (e.g., IPv4, PortsScan, CVE, Credentials, Vulnerability) so downstream finding-based attack-path chaining can consume the data (per the nmap/netexec/censys reference pattern).
Changes:
- Shodan: declares finding-compatible outputs for all query contracts and adds a
ShodanFindingsParserto extract IPv4 / PortsScan / CVE findings from Shodan matches on every execution. - AWS: upgrades key outputs from generic
Textto semantic finding types (Credentials, Vulnerability, IPv4, Port) and adds a Pacu-stdout semantic parser plus unit tests. - Adds/extends tests to lock in per-contract Shodan finding declarations and validate Shodan/AWS finding extraction behavior.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shodan/tests/unit/test_findings_parser.py | Unit tests for Shodan finding extraction (IPv4/PortsScan/CVE, asset mapping, dedup). |
| shodan/tests/shodan_contracts/test_finding_outputs.py | Ensures every Shodan contract declares the expected finding-compatible outputs. |
| shodan/shodan/services/findings_parser.py | New parser that converts Shodan matches into finding-compatible outputs. |
| shodan/shodan/services/init.py | Exports ShodanFindingsParser from the services package. |
| shodan/shodan/injector/openaev_shodan.py | Always emits finding-compatible structured outputs (in addition to optional auto-create-assets output). |
| shodan/shodan/contracts/ip_enumeration/contract.py | Declares PortsScan/IPv4/CVE outputs for the contract. |
| shodan/shodan/contracts/domain_discovery/contract.py | Declares IPv4/PortsScan outputs for the contract. |
| shodan/shodan/contracts/cve_specific_watchlist/contract.py | Declares CVE/PortsScan/IPv4 outputs for the contract. |
| shodan/shodan/contracts/cve_enumeration/contract.py | Declares CVE/PortsScan/IPv4 outputs for the contract. |
| shodan/shodan/contracts/custom_query/contract.py | Declares PortsScan/IPv4/CVE outputs for the contract. |
| shodan/shodan/contracts/critical_ports_and_exposed_admin_interface/contract.py | Declares PortsScan/IPv4/CVE outputs for the contract. |
| shodan/shodan/contracts/cloud_provider_asset_discovery/contract.py | Declares IPv4 output for the contract. |
| shodan/shodan/contracts/finding_outputs.py | Centralized builders/constants for Shodan finding-compatible contract outputs. |
| aws/test/test_pacu_executor_findings.py | New unit tests covering AWS Pacu semantic finding parsing. |
| aws/test/init.py | Initializes the AWS test package. |
| aws/aws/helpers/pacu_executor.py | Adds semantic parsing for EC2/VPC (IPv4/Port), Secrets/SSM (Credentials), IAM privesc (Vulnerability). |
| aws/aws/contracts_aws.py | Updates contract outputs to semantic finding types and adds IPv4/Port outputs to relevant contracts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #421 +/- ##
==========================================
- Coverage 81.58% 77.63% -3.96%
==========================================
Files 153 158 +5
Lines 6028 6692 +664
==========================================
+ Hits 4918 5195 +277
- Misses 1110 1497 +387
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
|
Review-and-fix pass complete.
|
Samuel Hassine (SamuelHassine)
left a comment
There was a problem hiding this comment.
Reviewed the full changed files for both injectors (not just the diff), plus the callers and the netexec/censys reference pattern.
The finding-output wiring is correct: output-dict keys match the platform OutputProcessor field contracts (PortsScan {host, port, service, asset_id}, CVE {id, host, severity, asset_id}, Credentials {username, hash}, Vulnerability {name, status, details}, IPv4 strings, Port ints), findings are validated before emission (valid IPv4 via ipaddress, integer ports in range, non-null required keys), asset_id is attached only when a match resolves to a target asset (never a null id), and dedup is applied on every finding type. The Shodan parser correctly handles both the dict and list forms of vulns and the matches/data response shapes.
One correctness fix applied on top (Copilot's catch): _extract_open_ports() no longer flattens a security-group range into its two endpoints; it emits a port only for single-port rules (FromPort == ToPort) and explicit "open port N" lines. Added tests for both cases and for skipping ranges.
Lint (black, flake8 --ignore=E,W) and the aws + shodan test suites pass locally and in CI. LGTM.
Shodan contracts fetched ports, IPs, hostnames and CVEs but emitted zero findings (only a non-finding-compatible Asset output). Every per-query contract now declares finding-compatible ContractOutputElement (IPv4, PortsScan, CVE) and a new ShodanFindingsParser turns the Shodan matches into those findings, wired into the injector output for every execution.
AWS enumeration emitted findings but everything was generic Text. High-value outputs now carry semantic types: Secrets Manager and SSM parameters emit Credentials, IAM privesc paths emit Vulnerability, and EC2 / VPC enumeration emit IPv4 (public IPs) and Port (open security-group ports). IAM Create User and S3 Download Bucket are left as-is by design.
All output-dict keys match the platform OutputProcessor field contracts: PortsScan {host, port, service, asset_id}, CVE {id, host, severity, asset_id}, Credentials {username, hash}, Vulnerability {name, status, details}, IPv4 primitive strings, Port primitive numbers.
Adds unit tests for the Shodan findings parser and per-contract output declarations, and for the AWS semantic finding parser.
…rts (#396) _extract_open_ports() extracted every number following FromPort, ToPort or Port, so a genuine security-group range such as "FromPort: 80 ToPort: 82" was emitted as the two disjoint ports [80, 82], dropping 81 and misrepresenting what is actually open. The contract output is a list of single Port primitives, so a range cannot be represented faithfully. The parser now only emits a security-group rule when FromPort == ToPort (a true single-port rule) and parses explicit "open port N" lines separately. The FromPort/ToPort pairing regex uses \D+ between the two values, so it can never pair a FromPort with a ToPort from a different rule. Adds tests for the single-port and explicit-line cases and for skipping multi-port ranges.
3614424 to
95e212b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Suppressed comments (4)
aws/aws/helpers/pacu_executor.py:1009
vpc__enum_lateral_movementprints progress and aggregate counts, not public IP values, so scanning its stdout here cannot populatepublic_ips. The contract now advertises an IPv4 finding that will stay empty in real executions; obtain addresses from an appropriate structured AWS/Pacu data source, or remove this output from the VPC contract.
elif "vpc" in module_name:
outputs["vpcs"] = self._extract_items(stdout, ["vpc-", "VPC:", "VpcId:"])
outputs["public_ips"] = self._extract_public_ipv4s(stdout)
aws/aws/helpers/pacu_executor.py:1072
- These prefixes do not match the real
secrets__enumoutput: Pacu printsFound secret: <name>and writes each<name>:<value>to its download artifact. As a result this parser returns no Secrets Manager findings during an actual run. Read the downloaded structured value (or otherwise expose module data) and build credentials from that instead of these synthetic stdout fixtures.
def _parse_secrets_credentials(self, stdout: str) -> List[Dict]:
"""Parse Secrets Manager identifiers into Credentials findings."""
identifiers = self._extract_items(stdout, ["SecretName:", "Secret:", "ARN:"])
return self._identifiers_to_credentials(identifiers)
aws/aws/helpers/pacu_executor.py:1077
systemsmanager__download_parametersdoes not print parameter names in this format; its stdout only reports the count and save path, while names and decrypted values are written to per-region JSON files. Therefore this parser returns an empty list in production despite the contract declaring Credentials. Consume the generated JSON/structured module result and emit the actual name/value pairs.
def _parse_ssm_credentials(self, stdout: str) -> List[Dict]:
"""Parse SSM parameter identifiers into Credentials findings."""
identifiers = self._extract_items(stdout, ["Parameter:", "Name:", "SSM:"])
return self._identifiers_to_credentials(identifiers)
aws/aws/helpers/pacu_executor.py:1094
- A secret or parameter identifier is not a credential hash. Copying the identifier into
hashonly satisfies the validator syntactically and sends unusable credential material into downstream reuse injects. Use the values Pacu actually downloads (for example, name asusernameand the secret value aspassword), or keep these outputs non-Credentials when no value is available.
credentials.append({"username": identifier, "hash": identifier})
…396) _parse_privesc_paths() matched any line containing "escalation", "privesc", "vulnerable" or "exploit", which also matches absence/failure summaries such as "No potential privilege escalation methods worked." - so a scan that found nothing was emitted as a VULNERABLE finding. Lines containing a negative marker (no potential, not vulnerable, none found, could not, unable to, ...) are now rejected before a Vulnerability finding is built. Adds tests covering the mixed affirmative/negative case and a negatives-only scan.
|
Rebased and re-reviewed.
Only remaining item: the required approval must come from another maintainer - GitHub blocks self-approval since I am the PR author, so it stays REVIEW_REQUIRED / BLOCKED although it is MERGEABLE with all checks green and all threads resolved. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
aws/aws/contracts_aws.py:638
- The VPC contract still declares only
vpcsandpublic_ips;_generic_parse()likewise adds noopen_portsfor VPC executions. This contradicts the PR description (and the new AWS test module documentation), which says VPC enumeration now emits both IPv4 and Port findings. Either implement and test VPC port extraction/declaration or narrow the stated scope.
[output_vpc_networks, output_public_ips],
aws/aws/helpers/pacu_executor.py:1124
- This fabricates a credential by using the secret/parameter identifier as its hash. Pacu writes the actual Secrets Manager
SecretStringand decrypted SSM values to download files; its console output exposes only names or counts/paths. As a result, Secrets Manager can emit bogusname:namecredentials and SSM normally emits none, so downstream credential-reuse chaining receives invalid data. Read the downloaded values (or otherwise plumb Pacu's result data) and emit a credential only when an actual password/hash value is available.
continue
seen.add(identifier)
credentials.append({"username": identifier, "hash": identifier})
Summary
Shodan contracts fetched ports, IPs, hostnames and CVEs but emitted zero findings, and AWS enumeration emitted findings that were all generic Text. This closes both gaps so the data can drive finding-based attack-path chaining, following the nmap / netexec / censys reference pattern.
Changes
P1 - Shodan (all seven query contracts): each contract now declares finding-compatible ContractOutputElement (IPv4, PortsScan, CVE) via shared builders in
shodan/shodan/contracts/finding_outputs.py, and a newShodanFindingsParser(shodan/shodan/services/findings_parser.py) turns the Shodan matches into those findings. Findings are emitted on every execution (not only when auto-create-assets is set), and an asset id is attached when a match maps back to a resolved target asset (ip / seen-ip / hostname), mirroring the netexec convention of never emitting a null asset id.P2 - AWS (
aws/aws/contracts_aws.py+aws/aws/helpers/pacu_executor.py): Secrets Manager (output_secrets) and SSM Parameters (output_ssm_parameters) now emit Credentials; IAM privesc paths (output_iam_privesc_paths) now emit Vulnerability; EC2 instances / security groups and VPC enumeration now additionally emit IPv4 (public IPs) and Port (open security-group ports). IAM Create User (no outputs) and S3 Download Bucket (raw json) are left unchanged by design.All output-dict keys match the platform OutputProcessor field contracts: PortsScan {host, port, service, asset_id}, CVE {id, host, severity, asset_id}, Credentials {username, hash}, Vulnerability {name, status, details}, IPv4 primitive strings, Port primitive numbers. Only findings that satisfy the platform validators are emitted (valid IPv4, integer ports, non-null required keys), so no dropped or garbage findings.
P3 (optional) is intentionally out of scope: http-query and stratus still emit no findings; left as follow-up.
Test plan
tests/unit/test_findings_parser.py(IPv4 / PortsScan / CVE extraction, asset mapping, dedup, error/non-IPv4 handling, list-form vulns) andtests/shodan_contracts/test_finding_outputs.py(per-contract finding-output declarations).test/test_pacu_executor_findings.py, a stdlib-only suite for the semantic finding parser (the AWS injector previously had no tests).black --checkandflake8 --ignore=E,Wclean on the touched injectors.Closes #396