Skip to content

Conversation

NicoHinderling
Copy link
Contributor

Instead of just returning the artifact id, lets return the full URL

@NicoHinderling NicoHinderling requested a review from a team as a code owner August 10, 2025 18:58
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 10, 2025
cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Aug 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #97558      +/-   ##
==========================================
+ Coverage   80.26%   80.64%   +0.37%     
==========================================
  Files        8561     8562       +1     
  Lines      377137   377168      +31     
  Branches    24548    24548              
==========================================
+ Hits       302698   304153    +1455     
+ Misses      74069    72645    -1424     
  Partials      370      370              

"""
organization: Organization = Organization.objects.get_from_cache(id=organization_id)

path = f"/organizations/{organization.slug}/preprod/internal/{artifact_id}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Cross-Silo Query Violation in URL Construction

The get_preprod_artifact_url function queries the Organization model (Organization.objects.get_from_cache) from a region-silo endpoint to build a URL (using organization.slug and absolute_url). As Organization is a control-silo model, this violates silo boundaries, risking cross-silo database access errors (e.g., SiloLimitError) in multi-silo deployments. The URL should instead be constructed using data already available (e.g., project.organization) or a silo-safe service/mapping (e.g., OrganizationMapping or an organization_service) to avoid querying the control database from the region.

Fix in Cursor Fix in Web

Copy link
Contributor

@chromy chromy left a comment

Choose a reason for hiding this comment

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

lgtm

@NicoHinderling NicoHinderling merged commit 8f0e25e into master Aug 11, 2025
64 checks passed
@NicoHinderling NicoHinderling deleted the artifact-url branch August 11, 2025 16:33
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
Instead of just returning the artifact id, lets return the full URL
priscilawebdev pushed a commit that referenced this pull request Aug 25, 2025
Instead of just returning the artifact id, lets return the full URL
@github-actions github-actions bot locked and limited conversation to collaborators Aug 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants