fix(recipes): resolve security-group direction from the Terraform state layer - #29
safwentrabelsi wants to merge 2 commits into
Conversation
…te layer The recipe told the reader direction was not modelled and to confirm in AWS. That is wrong. Terraform state models ingress and egress as separate resource types, so the graph carries them as distinct labels — AWS_VPC_SECURITY_GROUP_ INGRESS_RULE and ..._EGRESS_RULE — joined to the cloud-scanned rule by MANAGES. Found by Claude using the plugin on anyshift_infrastructure: it separated the two and reported 14 genuine internet-facing ingress rules. Verified: the 63 wide-open rules are 14 ingress, 25 egress and 24 unknown, and 14 is exactly right. The recipe now returns `direction` and says only ingress is exposure. 'unknown' means the rule has no state node (console-created, or managed outside Terraform), not that it is inbound — the two description/protocol heuristics stay, demoted to triage for that case only. Re-verified verbatim from the file: 50 rows, 1.6 s, 14 ingress / 25 egress / 11 unknown within the LIMIT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The direction column resolves from Terraform state. A project without the integration has no state nodes, so every row reads 'unknown' — that is the column being unavailable, not every rule being suspicious. Says to check describe_schema for the label before relying on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![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: 667734d965
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| count(DISTINCT t) AS attachedCount, | ||
| collect(DISTINCT coalesce(t.name, t.hashedID))[0..5] AS attachedTo | ||
| ORDER BY attachedCount DESC, securityGroup LIMIT 50 | ||
| ORDER BY direction, attachedCount DESC, securityGroup LIMIT 50 |
There was a problem hiding this comment.
In any project with at least 50 wide-open egress rules, ascending lexical ordering places every egress row before every ingress row, so LIMIT 50 can omit all actual exposures. Since the guidance below treats only ingress as actionable, this security recipe can produce a false-negative result; use an explicit priority that puts ingress first, filter by direction, or paginate each direction separately.
Useful? React with 👍 / 👎.
| // Direction comes from the Terraform-state layer, which models ingress and egress | ||
| // as separate resource types. Rules AWS created outside Terraform have no state | ||
| // node and stay 'unknown'. | ||
| OPTIONAL MATCH (st)-[:MANAGES]->(r) |
There was a problem hiding this comment.
When a Terraform resource has been removed while its AWS rule survives, the deleted state node retains its MANAGES relationship, as documented by the current-state invariant in skills/agent-plugin/SKILL.md. Because this match omits :ALIVE, the recipe treats that historical node as current Terraform evidence and bypasses the required unknown-direction verification; constrain st to live nodes.
Useful? React with 👍 / 👎.
1 participant
The recipe was wrong
Shipped an hour ago in #27, it said:
That is true of the cloud-scanned
EC2_SECURITYGROUPRULE, and false of the graph. Terraform state models the two as separate resource types, so the graph carries them as distinct labels joined to the cloud rule byMANAGES:AWS_VPC_SECURITY_GROUP_INGRESS_RULE— 165 nodesAWS_VPC_SECURITY_GROUP_EGRESS_RULE— 65 nodesFound by Claude using the plugin on
anyshift_infrastructure: it went past the caveat, used the state layer, and reported 14 genuine internet-facing ingress rules. Verified, and 14 is exactly right.What changed
The query returns
direction, and the caveat now says onlyingressis exposure.unknownmeans the rule has no Terraform-state node — console-created or managed elsewhere — not that it is inbound; theruleDescriptionandproto = '-1'heuristics stay, demoted to triage for that case alone.Re-verified verbatim from the file: 50 rows in 1.6 s, 14 ingress / 25 egress / 11 unknown within the LIMIT.
npm test40/40, validator passes. No version bump here — say the word and I cut v0.3.3, since v0.3.2 shipped the wrong caveat to Codex users.🤖 Generated with Claude Code