Skip to content

new root network default applicability - #1124

Open
ghazwarhili wants to merge 3 commits into
mainfrom
init-root-network-tag-applicability
Open

ghazwarhili wants to merge 3 commits into
mainfrom
init-root-network-tag-applicability

Conversation

@ghazwarhili

Copy link
Copy Markdown
Contributor

PR Summary

apply new root network default applicability

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4aeb42d8-877e-4893-a0af-b9391dba8299

📥 Commits

Reviewing files that changed from the base of the PR and between b23409b and ed82e9f.

📒 Files selected for processing (4)
  • src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java
  • src/main/java/org/gridsuite/study/server/service/StudyService.java
  • src/test/java/org/gridsuite/study/server/rootnetworks/RootNetworkApplicabilityTest.java
  • src/test/java/org/gridsuite/study/server/service/NetworkModificationServiceTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Root-network creation now initializes tag applicability for the study’s modification groups before creating the root network. The request includes the new tag, existing root-network tags, and group UUIDs.

Changes

Root-network tag applicability

Layer / File(s) Summary
Tag initialization request
src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java, src/test/java/org/gridsuite/study/server/service/NetworkModificationServiceTest.java
Adds query-parameter constants and a method that sends a POST to initialize a tag for modification groups. The rename request uses the NEW_TAG constant. Lombok @Setter replaces the explicit URI setter. The test checks the request and verifies that empty groups cause no further REST interaction.
Root-network creation integration
src/main/java/org/gridsuite/study/server/service/StudyService.java, src/test/java/org/gridsuite/study/server/rootnetworks/RootNetworkApplicabilityTest.java
Before creating a root network, gathers existing root-network tags and initializes the requested tag across the study’s modification groups. The test verifies the existing and new tags and the modification group passed to the service.

Sequence Diagram(s)

sequenceDiagram
  participant StudyService
  participant NetworkModificationService
  participant ModificationServer
  StudyService->>NetworkModificationService: initRootNetworkTag(group UUIDs, existing tags, new tag)
  NetworkModificationService->>ModificationServer: POST tag applicability request
  ModificationServer-->>NetworkModificationService: Response
  NetworkModificationService-->>StudyService: Return
  StudyService->>StudyService: Create root network
Loading

Suggested reviewers: souissimai

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to ed82e

No actionable issue is established for this change; it is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to ed82e

Creating a root network now changes modification applicability before creation is committed. Permission enforcement for shared modifications and recovery after partial failure remain unproven. These are meaningful design risks, although an authorization bypass has not been verified.

Retained concerns

  • Low · security · inferred: The new initialization path submits every study modification group without carrying the initiating user identity or applying the local shared-modification writability check used for renaming. If the remote endpoint lacks equivalent enforcement, creating a root network could mutate shared applicability beyond the caller's authority. This is an unresolved authorization contract, not a verified bypass.
  • Medium · reliability · inferred: A successful remote initialization followed by local persistence, link creation, or commit failure can leave applicability changed without a committed root network. The inspected callback logs failures without compensating this new side effect. Recovery and repeated initialization depend on unavailable remote semantics, weakening failure containment for shared mutable applicability state.
Security review details

Security Blast Radius

  • inferred — The initialization request covers every modification group selected from the study, not just one node. If it rewrites shared applicability records, effects may propagate to other studies referencing those modifications. The remote storage semantics and actual reference graph are missing, so tenant count and maximum independently attackable scope cannot be established.

Security Findings and Attack Paths

  • inferred — The deferred authorization candidate follows a caller-selected root-network tag through the pending request and import callback into bulk applicability initialization. An unauthorized-write outcome requires the remote endpoint to lack equivalent authority enforcement. No retained finding or verified authorization bypass is established.

Trust Boundaries and Controls

  • observed — The request stores studyUuid and userId, but initialization forwards only group and tag values with a content-type header. Existing rename checks carry userId to the references-authorized endpoint, and the separate modification-update route now forwards userId. Neither control is reused by initialization.

Resilience and Maintainability Implications

  • observed — The existing cancellation path deletes a pending request, and completion without a pending request takes a resource-cleanup branch rather than initializing applicability. These paths do not provide compensation for initialization that succeeds before a later creation failure. Callback failures are logged without an applicability recovery action in the inspected handler.

Hardening Proposals

  • proposed — Define the initialization authority contract explicitly: either enforce the initiating identity's permissions for affected shared modifications at mutation time, or establish a trusted lifecycle operation with equivalent ownership restrictions.
  • proposed — Give the cross-service transition a stable operation identity and documented atomicity, replay, and reconciliation behavior so interruption, concurrent creation, cancellation, or local commit failure cannot strand unowned applicability changes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: applying default applicability to new root networks.
Description check ✅ Passed The description directly states that the pull request applies new root network default applicability.
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.
  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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.

1 participant