Skip to content

fix(terraform): redact the value paired with a secret-named name in name/value lists - #3870

Closed
breken-ai wants to merge 1 commit into
Graphify-Labs:v8from
breken-ai:fix/terraform-name-value-secret
Closed

breken-ai wants to merge 1 commit into
Graphify-Labs:v8from
breken-ai:fix/terraform-name-value-secret

Conversation

@breken-ai

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #3787, a follow-up to #3817.

What: a Terraform { name = "DB_PASSWORD", value = "hunter2" } pair puts hunter2 in graph.json and the MCP query/get_node output verbatim. That is the usual shape for ECS environment lists and for module inputs such as environment = [...].

Why: _redact_value decides by the map KEY. Here the keys are the generic name and value, and the secret signal is the name literal, so nothing matched.

Small example:

  • environment = [{ name = "DB_PASSWORD", value = "hunter2" }, { name = "LOG_LEVEL", value = "debug" }]
  • On v8 it is stored as-is.
  • With this fix it is stored as [{name: DB_PASSWORD, value: [redacted]}, {name: LOG_LEVEL, value: debug}].

Why the existing tests missed it: every redaction test puts the secret word in a key (password, client_secret, var.db_password). None puts it in a value.

Fix: when a map has a name key whose string value matches _SENSITIVE_KEY_RE, redact its value / valueFrom / value_from too. This follows your suggestion in the issue and stays conservative:

  • An ordinary pair such as LOG_LEVEL is untouched.
  • The name stays visible, so you can still see which secret is set.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

Invariant: a credential literal in a .tf file never reaches graph.json or the MCP surface, while non-secret attributes pass through unchanged. The change only affects which attributes values are replaced by [redacted]. Nothing persisted is invalidated, and the next graphify update re-extracts affected .tf files.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

Limitations:

  • Only a literal name key is used as the pair signal. key/value shapes are not treated as pairs.
  • Pairs inside jsonencode(...) or heredoc values are stored as raw text, which is a separate gap. I have a separate small fix for it and can open it after this one.

How was this tested?

# regression (tests/test_terraform.py::test_terraform_name_value_pair_secret_is_redacted)
v8 4377ee9:   FAILED  assert {'name': 'DB_PASSWORD', 'value': 'hunter2'} == {'name': 'DB_PASSWORD', 'value': '[redacted]'}
this branch:  passed

# end to end: `graphify update .` on a directory holding the ECS example above
v8 4377ee9:   grep -c hunter2 graphify-out/graph.json -> 1   ("debug" present)
this branch:  grep -c hunter2 graphify-out/graph.json -> 0   ("debug" still present)

uv run pytest tests/test_terraform.py tests/test_terraform_modules.py -q   -> 41 passed
uv run pytest tests/ -q                                                    -> 6040 passed, 14 skipped (v8 baseline: 6039 passed, 14 skipped)
uv run ruff check graphify/extractors/terraform.py tests/test_terraform.py -> All checks passed
uv run pyright graphify/extractors/terraform.py                           -> no new errors (same count as v8)

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments. (n/a: no skill sources touched)
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables).
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

AI disclosure: an AI agent (Claude, operating the breken-ai account) found and wrote this fix. I verified the red/green regression, the end-to-end graphify update reproduction, the full suite, ruff and pyright as listed above. The commit carries a Co-Authored-By: Claude trailer per CONTRIBUTING.

…ame/value lists

The ECS environment idiom { name = "DB_PASSWORD", value = "hunter2" } names the
secret in the name literal, not in a key, so key-name redaction let the value
reach graph.json and the MCP surface verbatim. When a map's name literal
matches the sensitive pattern, redact its value/valueFrom.

Fixes Graphify-Labs#3787

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @breken-ai. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

Redacts secrets in the name/value-pair idiom (ECS environment/secrets, any [{name, value}] list): when _redact_value sees a dict whose name literal matches the sensitive-key pattern, it now redacts the paired value/valueFrom/value_from entry, closing a leak where { name = "DB_PASSWORD", value = "hunter2" } reached graph.json verbatim. Non-secret pairs like { name = "LOG_LEVEL", value = "debug" } pass through untouched.

No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1460 functions depend on the 54 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 711 callers, 48 callees
  • new: _rebuild_code() — 144 callers, 55 callees
  • new: main() — 98 callers, 3 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • new: extract_terraform() — 23 callers, 8 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: watch() — 5 callers, 7 callees
  • new: _build() — 7 callers, 3 callees
  • …and 10 more — each is listed as a finding

Verification — 1460 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 795 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

2 of 302 test file(s) selected (1%) via static blast radius.

  • tests/test_terraform.py — impact, changed-test
  • tests/test_terraform_modules.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

· 18 more finding(s) on lines outside this diff (see the check run).

safishamsi added a commit that referenced this pull request Sep 27, 2026
Security: close the Fortran cpp #include arbitrary-file-read
(GHSA-pcc4-rvhr-2pr8), the last Aider/Devin monolith --watch shell sink
(#3852), and terraform name/value secret redaction (#3870). Plus Windows
watch rebuild locking (#3883), C# tuple element-name refs (#3877), JSX
component-usage calls (#3855), and nested scan-root Python import
projection (#3867).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Member

Shipped in v0.9.70 (now on PyPI) via an authorship-preserving cherry-pick, so your commit keeps contributor-graph credit. Thanks @breken-ai! Redacts a value paired with a secret-named name in name/value pair lists.

Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.70

@safishamsi safishamsi closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terraform secret redaction misses the name/value-pair idiom (AWS environment/secrets lists)

2 participants