Skip to content

test: [TESIS-161] make the tenant isolation sweep cover every route with an id - #114

Merged
LauAubert merged 2 commits into
masterfrom
TESIS-999032-isolation-sweep-covers-every-id-route
Oct 5, 2026
Merged

LauAubert merged 2 commits into
masterfrom
TESIS-999032-isolation-sweep-covers-every-id-route

Conversation

@LauAubert

@LauAubert LauAubert commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Ticket de Jira

https://proyectofinalfrlp.atlassian.net/browse/TESIS-161

ID provisorio: se reemplaza por la clave real al cargar la card en Jira. Card: cards/032.md.


Descripción

El criterio de aceptación de E4b §2.6 pide que «ningún tenant pueda acceder a datos de otro tenant bajo ningún escenario de prueba». tenant_isolation_spec.rb lo prueba barriendo rutas con un id de otra empresa y esperando 404, pero la lista de rutas está escrita a mano: una ruta nueva sólo quedaba cubierta si alguien se acordaba de sumarla. Revisándola contra el router, faltaban tres:

  • POST /api/v1/shipments/:id/dispatch
  • POST /api/v1/products/:product_id/mappings
  • DELETE /api/v1/products/:product_id/mappings/:id

Las tres responden 404 con un id ajeno: no hay fuga, pero no estaban probadas.

  • Suma las tres al barrido (con un other_mapping de la otra empresa para la baja).
  • Agrega «sweeps every route of the API that takes an id»: toma del router toda ruta de /api/v1 cuyo path lleva un id (:id, :order_id, :product_id, :service_id) y falla, nombrándola, si no está en el barrido. integrations#update queda listada aparte: su id es una plantilla global (Service), y la integración que toca es siempre la de la empresa de la sesión.

Dientes: sacando la línea de dispatch del barrido, el ejemplo nuevo falla con expected ["shipments#confirm"].empty? to be truthy.

Orden de merge: proyecto-api#102 (card 009) agrega POST /orders/:id/cancel; ya sumé esa ruta al barrido en su propia rama, así que mergear una antes que la otra no rompe nada (a lo sumo, un conflicto de líneas vecinas en la lista).


Evidencia visual

N/A


Cómo probar

  1. bundle exec rspec spec/requests/api/v1/tenant_isolation_spec.rb → en verde.
  2. Sacar cualquier ruta del barrido → falla el ejemplo nuevo nombrando controlador#acción.

Verificación: bundle exec rspec (1568 ejemplos, 0 fallas), rubocop limpio.


Impacto y consideraciones

¿Introduce breaking changes?
No. Sólo tests.

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No. Mismo criterio que la card 031 para ADR-015: la regla se verifica contra el router y no contra una lista.

🤖 Generated with Claude Code

…e with an id

The isolation spec sweeps a hand-written list of routes that receive an id
of another company and expects 404 on each. A new route was only covered if
someone remembered to add it: POST /shipments/:id/dispatch and the creation
and deletion of product mappings were never swept.

Those three are added (all answer 404), and a new example takes every
/api/v1 route whose path carries an id from the router and fails, naming
it, when one is not in the sweep. Routes whose id is not a record of a
company (the global service of integrations#update) are listed apart.

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

Copy link
Copy Markdown
Contributor

Revisado. Lo veo bien para implementar.

Lo mergearía por lo que encontró: POST /api/v1/shipments/:id/dispatch, POST /api/v1/products/:product_id/mappings y DELETE .../mappings/:id estaban fuera del barrido de aislamiento. Son tres rutas que mueven stock o tocan el vínculo con un canal, y el criterio de aceptación de E4b §2.6 pide que ningún tenant alcance datos de otro bajo ningún escenario de prueba. Que faltaran no quiere decir que fueran vulnerables —el default_scope de CompanyScoped las cubre igual—, pero sí que nadie lo estaba probando, que es lo que el criterio pide demostrar.

Lo que verifiqué

  • El ejemplo sweeps every route of the API that takes an id cierra el agujero de raíz: usa recognize_path sobre las rutas del barrido y las compara contra las que el router expone con un id. Una ruta nueva con id que nadie agregue hace fallar el spec y la nombra. Eso es lo que convierte esto en una garantía y no en otra lista que se va a quedar vieja.
  • tenantless_id_routes = %w[integrations#update] está bien justificado: el :service_id apunta a una plantilla global y la integración que se actualiza es siempre la de la sesión. Es la única excepción y está escrita como excepción, no escondida dentro del filtro.
  • other_mapping se crea dentro de as_intruder, igual que el resto de los fixtures ajenos, así que nace con el company_id de la otra empresa y no con el que dejó colgado un request anterior.

Antes de mergear

Igual que #113: scan_ruby en rojo es la corrida vieja del 02/10 y el PR sólo toca spec/. Rebasá sobre master y vuelve a correr.

Lo tomaría como parte del trabajo de QA del tablero, no como una card de desarrollo.

@LauAubert LauAubert changed the title test: [TESIS-999032] make the tenant isolation sweep cover every route with an id test: [TESIS-161] make the tenant isolation sweep cover every route with an id Oct 3, 2026
@LauAubert LauAubert closed this Oct 3, 2026
@LauAubert
LauAubert deleted the TESIS-999032-isolation-sweep-covers-every-id-route branch October 3, 2026 23:19
@LauAubert
LauAubert restored the TESIS-999032-isolation-sweep-covers-every-id-route branch October 3, 2026 23:23
@LauAubert LauAubert reopened this Oct 3, 2026
@LauAubert
LauAubert marked this pull request as ready for review October 3, 2026 23:25
@LauAubert
LauAubert requested a review from a team as a code owner October 3, 2026 23:25
@LauAubert
LauAubert requested a review from LoLoo03 October 3, 2026 23:25

@TomasMartin2004 TomasMartin2004 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.

Revisión del diff completo. El código es idéntico al que leí cuando los PRs estaban en draft —ningún commit nuevo—, así que lo que sigue es el veredicto formal.

✅ Aprobado

Es el PR con mejor relación valor/costo de la tanda, junto con #113: 41 líneas, todas de spec, y cubre una de las tareas transversales de QA que siguen abiertas.

Lo que encontró es el punto: POST /shipments/:id/dispatch, POST /products/:product_id/mappings y DELETE .../mappings/:id estaban fuera del barrido de aislamiento. Son tres rutas que mueven stock o tocan el vínculo con un canal, y el criterio de aceptación de E4b §2.6 pide que ningún tenant alcance datos de otro bajo ningún escenario de prueba. Que faltaran no quiere decir que fueran vulnerables —el default_scope de CompanyScoped las cubre— pero sí que nadie lo estaba demostrando, que es lo que el criterio pide.

Lo que verifiqué

  • El ejemplo sweeps every route of the API that takes an id cierra el agujero de raíz: usa recognize_path y compara contra lo que el router expone. Una ruta nueva con id que nadie sume hace fallar el spec y la nombra. Eso es lo que lo convierte en una garantía y no en otra lista que se va a quedar vieja.
  • tenantless_id_routes = %w[integrations#update] es la única excepción y está escrita como excepción, no escondida en el filtro. El :service_id apunta a una plantilla global y la integración que se actualiza es siempre la de la sesión.
  • other_mapping nace dentro de as_intruder, así que tiene el company_id de la otra empresa y no el que dejó colgado un request anterior.

Sin objeciones. Lo tomaría como trabajo de QA del tablero, no como una card de desarrollo.

@LauAubert
LauAubert merged commit ccd1b95 into master Oct 5, 2026
4 checks passed
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.

2 participants