Skip to content

fix(api): bound attack paths normalized-list child IDs#11960

Open
josema-xyz wants to merge 1 commit into
masterfrom
fix/attack-paths-bounded-child-ids
Open

fix(api): bound attack paths normalized-list child IDs#11960
josema-xyz wants to merge 1 commit into
masterfrom
fix/attack-paths-bounded-child-ids

Conversation

@josema-xyz

@josema-xyz josema-xyz commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Context

Attack Paths materializes configured list properties as child nodes. Their _provider_element_id previously included the complete normalized value.

Large values could exceed Neo4j's RANGE index key size limit and cause graph synchronization to fail. The generated identifier is shared by the Neo4j and Neptune sinks, so it must remain deterministic and bounded.

Description

  • Hash the existing deterministic value_key with SHA-256 when generating normalized-list child IDs
  • Preserve the provider ID and child label as identity scopes
  • Keep the complete original value in the child node properties
  • Keep parent IDs, relationships, sink implementations, indexes, and Attack Paths queries unchanged
  • Add regression coverage for large values, deterministic IDs, identity scopes, and relationship endpoints

No new dependencies are required.

Steps to review

  1. Review _build_child_id() in tasks/jobs/attack_paths/sync.py.
  2. Confirm normalized-list values remain available in child node properties.
  3. Confirm parent-child relationships use the hashed child ID.
  4. Run the focused regression tests:
    cd api
    uv run pytest src/backend/tasks/tests/test_attack_paths_scan.py -k TestBuildChildId
  5. Confirm both Neo4j and Neptune continue consuming the same transformed provider_element_id.

Checklist

API

  • All issue/task requirements work as expected on the API
  • Endpoint response output (if applicable)
  • EXPLAIN ANALYZE output for new/modified queries or indexes (if applicable)
  • Performance test results (if applicable)
  • Any other relevant evidence of the implementation (if applicable)
  • Verify if API specs need to be regenerated.
  • Check if version updates are required (e.g., specs, uv, etc.).
  • Ensure a changelog fragment is added under api/changelog.d/, if applicable.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Attack Paths scans to handle very large normalized list values without exceeding database index key limits.
    • Preserved original values while using bounded identifiers for better scan reliability.
  • Tests

    • Added coverage for large values and identifier uniqueness across different identity components.

@josema-xyz josema-xyz requested a review from a team as a code owner July 13, 2026 14:53
Copilot AI review requested due to automatic review settings July 13, 2026 14:53
@josema-xyz josema-xyz added the backport-to-v5.33 Backport PR to the v5.33 branch label Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

No Conflicts

No conflict markers, and the branch merges cleanly into its base.

@github-actions

Copy link
Copy Markdown
Contributor

✅ All required changelog fragments are present.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9ae76434-62d9-4297-840f-f0b50538d3d8

📥 Commits

Reviewing files that changed from the base of the PR and between 488f3d1 and 87ea841.

📒 Files selected for processing (3)
  • api/changelog.d/attack-paths-child-id-size.fixed.md
  • api/src/backend/tasks/jobs/attack_paths/sync.py
  • api/src/backend/tasks/tests/test_attack_paths_scan.py

📝 Walkthrough

Walkthrough

Attack Path scans now hash normalized list value keys when generating child node identifiers. New tests verify bounded IDs, preservation of raw values in child properties, and identifier changes when identity components differ. The changelog documents the Neo4j and Neptune behavior.

Changes

Attack Path child identifier sizing

Layer / File(s) Summary
Bounded child ID generation and validation
api/src/backend/tasks/jobs/attack_paths/sync.py, api/src/backend/tasks/tests/test_attack_paths_scan.py, api/changelog.d/attack-paths-child-id-size.fixed.md
_build_child_id uses a SHA-256 digest for normalized list value keys; tests cover large values, preserved child data, and identity uniqueness, while the changelog records the update.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: bounding Attack Paths normalized-list child IDs.
Description check ✅ Passed The description follows the template with context, summary, review steps, checklist, and license.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/attack-paths-bounded-child-ids

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a graph-sync failure mode in Attack Paths by bounding normalized-list child node _provider_element_id values so they can’t exceed Neo4j RANGE index key size limits, while keeping IDs deterministic and consistent across Neo4j and Neptune sinks.

Changes:

  • Update normalized-list child IDs to use a SHA-256 digest of the existing deterministic value_key (while retaining provider ID and child label as scopes).
  • Preserve the full original normalized-list value in the child node properties (props.value).
  • Add regression tests covering large values, determinism, identity scoping, and relationship endpoints; add an API changelog fragment.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
api/src/backend/tasks/jobs/attack_paths/sync.py Hashes value_key when building normalized-list child node IDs to keep identifiers bounded/deterministic.
api/src/backend/tasks/tests/test_attack_paths_scan.py Adds regression tests validating hashed child IDs, preserved values, determinism, and relationship wiring.
api/changelog.d/attack-paths-child-id-size.fixed.md Documents the fix preventing Neo4j RANGE index key size failures during Attack Paths sync.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

Copy link
Copy Markdown
Contributor

🔒 Container Security Scan

Image: prowler-api:98470be
Last scan: 2026-07-13 14:58:40 UTC

✅ No Vulnerabilities Detected

The container image passed all security checks. No known CVEs were found.

📋 Resources:

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.93%. Comparing base (1060266) to head (87ea841).
⚠️ Report is 16 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11960   +/-   ##
=======================================
  Coverage   93.93%   93.93%           
=======================================
  Files         266      266           
  Lines       39613    39634   +21     
=======================================
+ Hits        37209    37230   +21     
  Misses       2404     2404           
Flag Coverage Δ
api 93.93% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
prowler ∅ <ø> (∅)
api 93.93% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-v5.33 Backport PR to the v5.33 branch component/api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants