informatics-isi-edu / deriva-mcp-core / 25091590128

90%

push

github

fix(rag): correct comment framing and add missing test coverage for per-RID filter

  The comment added in 55e6eec framed the trailing ":" in own_data_per_rid_prefix
  as a security defense against a "malicious user_id" causing prefix-overlap.
  That framing is wrong on two counts.

  First, the threat model is not real: user IDs are assigned by the IDP (Keycloak,
  Globus, etc.) and callers do not choose them, so a user cannot craft an ID that
  is a deliberate prefix of another user's ID. Any overlap would be coincidental.

  Second, and more fundamentally, the ":" is not a defensive add-on -- it is the
  correct structural check by construction. The per-RID source format is
  data:{hostname}:{catalog_id}:{user_id}:{schema}:{table}:{rid}, where user_id can
  itself contain ":" (URL-form OIDC subs, Globus identifiers). That means the
  string cannot be split on ":" to extract fields; own_data must be matched as a
  whole. startswith(own_data + ":") is then the only unambiguous way to confirm
  that a source is own_data extended with additional colon-separated components
  rather than a string that merely starts with it. The trailing colon is the next
  structural delimiter, not a guard against an attack.

  Rewrite the production comment to explain the actual constraint. Rename
  test_data_results_per_rid_prefix_overlap_does_not_leak to
  test_data_results_per_rid_cross_user_isolation and remove the attack framing
  from its inline comment. Add test_data_results_per_rid_user_id_with_colons,
  which exercises a Globus URL-form user_id -- the case the whole design exists
  to handle and that was previously untested.

  Also expand the rag_search docstring to enumerate all four indexed source
  types (documentation, schema, catalog-data, enriched) with their source
  naming conventions and per-type scoping behavior. The previous docstring
  mentioned only documentation and schemas, leaving data: and enriched:
  sources undocumented.

  Rename the resolv... (continued)

1 of 1 new or added line in 1 file covered. (100.0%)

3169 of 3517 relevant lines covered (90.11%)

0.9 hits per line