Skip to content

fix(Dependencies): Dependency overrides serve a stale value while the prerequisite is off - #8669

Open
Ishita-Singh-12 wants to merge 5 commits into
Flagsmith:mainfrom
Ishita-Singh-12:Ishita-Singh-12-patch-4
Open

Ishita-Singh-12 wants to merge 5 commits into
Flagsmith:mainfrom
Ishita-Singh-12:Ishita-Singh-12-patch-4

Conversation

@Ishita-Singh-12

Copy link
Copy Markdown

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to #8628

A dependency override keeps the value and variants the feature had when the dependency was created. While the prerequisite isn't enabled, SDKs serve that stale value.

  • Dependency overrides now take their value and variants from the feature's current environment default. enabled, priority and metadata still come from the override.
  • Both map_environment_to_evaluation_context and map_environment_to_engine do this, using the new get_dependency_segment_ids to find the segments. It matches system segments owned by a feature that have flag references, so experiment rollout segments are left alone.

Not in this PR: showing the value as inherited in the admin UI. There is no UI for dependencies on main yet, so that would go with the Dependencies tab.

How did you test this code?

Added tests/unit/features/dependencies/test_overrides.py, which covers both mappers:

  • After changing the default value, the dependency override has the new value, is still disabled and keeps priority 0. Fails on main.
  • After changing the default's variant allocation, the dependency override has the same variants. Fails on main.
  • A system segment that isn't a dependency keeps its own value.

Ran the full unit suite and the integration suite. Two feature lifecycle tests error here because they need an InfluxDB service. ruff and mypy are clean.

@Ishita-Singh-12
Ishita-Singh-12 requested a review from a team as a code owner October 4, 2026 18:08
@Ishita-Singh-12
Ishita-Singh-12 requested review from emyller and removed request for a team October 4, 2026 18:08
@vercel

vercel Bot commented Oct 4, 2026

Copy link
Copy Markdown

@Ishita-Singh-12 is attempting to deploy a commit to the Flagsmith Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the api Issue related to the REST API label Oct 4, 2026
@coderabbitai

coderabbitai Bot commented Oct 4, 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: ASSERTIVE
  • Plan: Advanced
  • Run ID: f7d77aab-f7a6-471f-ab53-c288dc71db45
📥 Commits

Reviewing files that changed from the base of the PR and between 759f724 and 226d6c0.

📒 Files selected for processing (4)
  • api/evaluation/mappers.py
  • api/features/dependencies/services.py
  • api/tests/unit/features/dependencies/test_overrides.py
  • api/util/mappers/engine.py

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


📝 Walkthrough

Walkthrough

Dependency segment overrides now use the corresponding environment default feature state as the source of the emitted value and multivariate allocations in both evaluation-context and engine mappings. Other system segment overrides continue to use their own values. The change adds a service function to identify dependency segment IDs and adds unit tests for both mapping paths.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 226d6

The dependency override mapping is mergeable after normal checks; no unresolved issue was established that requires a fix before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 226d6

The change is bounded to dependency override values. Environment scoping, prerequisite gating, override identity and precedence are preserved. No introduced security weakness was established, but consistency during concurrent edits and cached or replicated reads remains incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed value exposure is bounded to dependency overrides participating in the evaluated environment. Shared project segment definitions do not themselves select values from another environment.

Trust Boundaries and Controls

  • observed — Persisted dependency references classify segments, while environment-scoped feature-state resolution supplies inherited data. Override control fields remain authoritative. Dependency creation also uses a transaction and project dependency lock before establishing the disabled override.

Resilience and Maintainability Implications

  • observed — The new mapping branch mutates only local dictionaries and output objects. Reading default allocations does not consume their dictionary entry, so later default-state mapping remains possible without introducing persistent cleanup or recovery state.

Hardening Proposals

  • proposed — Validate and document the expected consistency of dependency classification, cached segment membership and replicated default-state reads during dependency edits. This would close the remaining concurrency and recovery evidence gap, rather than remediate a demonstrated bypass.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@Ishita-Singh-12 Ishita-Singh-12 changed the title fix(Dependencies): dependency overrides serve a stale value while the prerequisite is off fix(Dependencies): Dependency overrides serve a stale value while the prerequisite is off Oct 4, 2026

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

api Issue related to the REST API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant