Enforce sequence coordinate bounds

src/ga4gh/vrs/dataproxy.py

@@ -16,6 +16,8 @@
 import requests
 from bioutils.accessions import coerce_namespace
 
+from ga4gh.vrs.models import Range
+
 _logger = logging.getLogger(__name__)
 
 
@@ -181,6 +183,56 @@ def validate_ref_seq(
             if require_validation:
                 raise DataProxyValidationError(err_msg)
 
+    def validate_location_bounds(
+        self,
+        sequence_id: str,
+        start_pos: int | Range | None,
+        end_pos: int | Range | None,
+    ) -> None:
+        """Ensure that ``start_pos`` and ``end_pos`` are representable on ``sequence_id``.
+
+        Each defined coordinate must be within ``[0, len(sequence)]`` (inter-residue).
+        Undefined (``None``) endpoints are skipped, and for a ``Range`` every defined
+        member is checked. ``start_pos`` and ``end_pos`` are checked independently, so
+        ``start_pos > end_pos`` (circular sequences) is permitted.
+
+        Unlike ``validate_ref_seq``, there is no ``require_validation`` option: an
+        out-of-bounds location has no meaning, so the error is always raised. Sequence
+        backends may silently truncate out-of-range fetches, so this check must be made
+        before relying on fetched sequence.
+
+        :param sequence_id: Sequence ID to use
+        :param start_pos: Start pos (inter-residue) on the sequence_id
+        :param end_pos: End pos (inter-residue) on the sequence_id
+        :raises DataProxyValidationError: If a defined coordinate is out of bounds
+        :raises KeyError: If ``sequence_id`` is not found
+        """
+        # same cache key as derive_refget_accession
+        sequence_id = coerce_namespace(sequence_id)
+        md = self.get_metadata(sequence_id)
+        seq_len = md["length"]
+        bad = []
+        for name, pos in (("start", start_pos), ("end", end_pos)):
+            values = pos.root if isinstance(pos, Range) else [pos]
+            if any(v is not None and not 0 <= v <= seq_len for v in values):
+                bad.append((name, pos))
+        if bad:
+            # Range is shown as its list form, e.g. end=[4500, 4600]
+            detail = ", ".join(
+                f"{name}={pos.root if isinstance(pos, Range) else pos}"
+                for name, pos in bad
+            )
+            # Name the sequence as given, plus the refget accession it resolved to
+            refget_ac = next((a for a in md["aliases"] if a.startswith("ga4gh:")), None)
+            seq_name = sequence_id
+            if refget_ac and refget_ac != sequence_id:
+                seq_name += f" ({refget_ac})"
+            err_msg = (
+                f"Location out of bounds on {seq_name}: {detail} "
+                f"not within [0, {seq_len}]"
+            )
+            raise DataProxyValidationError(err_msg)
+
 
 class _SeqRepoDataProxyBase(_DataProxy):
     # wraps seqreqpo classes in order to provide translation to/from

src/ga4gh/vrs/extras/translator.py

@@ -203,6 +203,8 @@ def _create_allele(self, values: dict, **kwargs) -> models.Allele:
 
         Args:
             values (dict): The values to use for creating the allele object.
+                'sequence_id' (str): The sequence identifier from the input
+                    expression, used to validate `start` and `end`.
                 'refget_accession' (str): The accession ID of the reference genome.
                 'start' (int): The start position of the allele.
                 'end' (int): The end position of the allele.
@@ -213,6 +215,9 @@ def _create_allele(self, values: dict, **kwargs) -> models.Allele:
             models.Allele: The created allele object.
 
         """
+        self.data_proxy.validate_location_bounds(
+            values["sequence_id"], values["start"], values["end"]
+        )
         seq_ref = models.SequenceReference(refgetAccession=values["refget_accession"])
         location = models.SequenceLocation(
             sequenceReference=seq_ref, start=values["start"], end=values["end"]
@@ -276,6 +281,7 @@ def _from_beacon(self, beacon_expr: str, **kwargs) -> models.Allele | None:
         ins_seq = alt
 
         values = {
+            "sequence_id": sequence,
             "refget_accession": refget_accession,
             "start": start,
             "end": end,
@@ -340,6 +346,9 @@ def _from_gnomad(self, gnomad_expr: str, **kwargs) -> models.Allele | None:
         ins_seq = alt
 
         # validation checks
+        # Bounds must be checked before the ref check: an out-of-bounds fetch may be
+        # silently truncated, which would be misreported as a reference mismatch
+        self.data_proxy.validate_location_bounds(sequence, start, end)
         self.data_proxy.validate_ref_seq(
             sequence,
             start,
@@ -349,6 +358,7 @@ def _from_gnomad(self, gnomad_expr: str, **kwargs) -> models.Allele | None:
         )
 
         values = {
+            "sequence_id": sequence,
             "refget_accession": refget_accession,
             "start": start,
             "end": end,
@@ -414,6 +424,7 @@ def _from_spdi(self, spdi_expr: str, **kwargs) -> models.Allele | None:
         ins_seq = g["ins_seq"]
 
         values = {
+            "sequence_id": g["ac"],
             "refget_accession": refget_accession,
             "start": start,
             "end": end,
@@ -570,12 +581,16 @@ def _from_hgvs(
         if not refget_accession:
             return None
 
+        start = sv.posedit.pos.start.base - 1
+        end = sv.posedit.pos.end.base
+        self.data_proxy.validate_location_bounds(sv.ac, start, end)
+
         location = models.SequenceLocation(
             sequenceReference=models.SequenceReference(
                 refgetAccession=refget_accession
             ),
-            start=sv.posedit.pos.start.base - 1,
-            end=sv.posedit.pos.end.base,
+            start=start,
+            end=end,
         )
 
         copies = kwargs.get("copies")

src/ga4gh/vrs/normalize.py

@@ -111,6 +111,8 @@ def _normalize_allele(
         of the `sequence`.
         To exclude `sequence` from the response, set to 0.
         For no limit, set to `None`.
+    :raises DataProxyValidationError: If the allele location is out of bounds on its
+        sequence
     """
     # Algorithm applies to LiteralSequenceExpression alleles only; other states are returned unchanged
     if not isinstance(input_allele.state, models.LiteralSequenceExpression):
@@ -130,6 +132,14 @@ def _normalize_allele(
 
     # 0: Get reference sequence and interval
     ref_seq = SequenceProxy(data_proxy, alias)
+
+    # Reject locations that do not exist on the sequence before anything is fetched,
+    # since out-of-range fetches may be silently truncated by the sequence backend.
+    # Done before the early returns below, which skip definite ranges.
+    data_proxy.validate_location_bounds(
+        alias, input_allele.location.start, input_allele.location.end
+    )
+
     start = _get_allele_location_pos(input_allele, use_start=True)
     if start is None:
         return input_allele

src/ga4gh/vrs/utils/hgvs_tools.py

@@ -182,6 +182,7 @@ def extract_allele_values(self, hgvs_expr: str) -> dict | None:
         (start, end, state) = self.get_position_and_state(sv)
 
         return {
+            "sequence_id": sv.ac,
             "refget_accession": refget_accession,
             "start": start,
             "end": end,

tests/extras/cassettes/test_in_bounds[spdi-insertion-at-end].yaml

@@ -0,0 +1,58 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/refseq:NM_000551.3
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-24T05:03:11Z\",\n  \"aliases\": [\n    \"MD5:215137b1973c1a5afcf86be7d999574a\",\n
+        \   \"NCBI:NM_000551.3\",\n    \"refseq:NM_000551.3\",\n    \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n
+        \   \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n    \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n
+        \   \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n    \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n
+        \ ],\n  \"alphabet\": \"ACGT\",\n  \"length\": 4560\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-24T05:03:11Z\",\n  \"aliases\": [\n    \"MD5:215137b1973c1a5afcf86be7d999574a\",\n
+        \   \"NCBI:NM_000551.3\",\n    \"refseq:NM_000551.3\",\n    \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n
+        \   \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n    \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n
+        \   \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n    \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n
+        \ ],\n  \"alphabet\": \"ACGT\",\n  \"length\": 4560\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/sequence/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_?start=4560&end=4560
+  response:
+    body:
+      string: ''
+    headers: {}
+    status:
+      code: 200
+      message: OK
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/sequence/ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_?start=4559&end=4560
+  response:
+    body:
+      string: G
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[beacon-past-end].yaml

@@ -0,0 +1,25 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-27T21:17:00Z\",\n  \"aliases\": [\n    \"GRCh38:1\",\n
+        \   \"GRCh38:chr1\",\n    \"GRCh38.p1:1\",\n    \"GRCh38.p1:chr1\",\n    \"GRCh38.p10:1\",\n
+        \   \"GRCh38.p10:chr1\",\n    \"GRCh38.p11:1\",\n    \"GRCh38.p11:chr1\",\n
+        \   \"GRCh38.p12:1\",\n    \"GRCh38.p12:chr1\",\n    \"GRCh38.p2:1\",\n    \"GRCh38.p2:chr1\",\n
+        \   \"GRCh38.p3:1\",\n    \"GRCh38.p3:chr1\",\n    \"GRCh38.p4:1\",\n    \"GRCh38.p4:chr1\",\n
+        \   \"GRCh38.p5:1\",\n    \"GRCh38.p5:chr1\",\n    \"GRCh38.p6:1\",\n    \"GRCh38.p6:chr1\",\n
+        \   \"GRCh38.p7:1\",\n    \"GRCh38.p7:chr1\",\n    \"GRCh38.p8:1\",\n    \"GRCh38.p8:chr1\",\n
+        \   \"GRCh38.p9:1\",\n    \"GRCh38.p9:chr1\",\n    \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n
+        \   \"NCBI:NC_000001.11\",\n    \"refseq:NC_000001.11\",\n    \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n
+        \   \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n    \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n
+        \   \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n    \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n
+        \ ],\n  \"alphabet\": \"ACGMNRT\",\n  \"length\": 248956422\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[cnv-hgvs-copy-number-change-past-end].yaml

@@ -0,0 +1,25 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/refseq:NC_000007.14
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-27T21:23:35Z\",\n  \"aliases\": [\n    \"GRCh38:7\",\n
+        \   \"GRCh38:chr7\",\n    \"GRCh38.p1:7\",\n    \"GRCh38.p1:chr7\",\n    \"GRCh38.p10:7\",\n
+        \   \"GRCh38.p10:chr7\",\n    \"GRCh38.p11:7\",\n    \"GRCh38.p11:chr7\",\n
+        \   \"GRCh38.p12:7\",\n    \"GRCh38.p12:chr7\",\n    \"GRCh38.p2:7\",\n    \"GRCh38.p2:chr7\",\n
+        \   \"GRCh38.p3:7\",\n    \"GRCh38.p3:chr7\",\n    \"GRCh38.p4:7\",\n    \"GRCh38.p4:chr7\",\n
+        \   \"GRCh38.p5:7\",\n    \"GRCh38.p5:chr7\",\n    \"GRCh38.p6:7\",\n    \"GRCh38.p6:chr7\",\n
+        \   \"GRCh38.p7:7\",\n    \"GRCh38.p7:chr7\",\n    \"GRCh38.p8:7\",\n    \"GRCh38.p8:chr7\",\n
+        \   \"GRCh38.p9:7\",\n    \"GRCh38.p9:chr7\",\n    \"MD5:cc044cc2256a1141212660fb07b6171e\",\n
+        \   \"NCBI:NC_000007.14\",\n    \"refseq:NC_000007.14\",\n    \"SEGUID:4+JjCcBVhPCr8vdIhUKFycPv8bY\",\n
+        \   \"SHA1:e3e26309c05584f0abf2f748854285c9c3eff1b6\",\n    \"VMC:GS_F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\",\n
+        \   \"sha512t24u:F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\",\n    \"ga4gh:SQ.F-LrLMe1SRpfUZHkQmvkVKFEGaoDeHul\"\n
+        \ ],\n  \"alphabet\": \"ACGNRSTY\",\n  \"length\": 159345973\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[gnomad-negative-start].yaml

@@ -0,0 +1,25 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-27T21:17:00Z\",\n  \"aliases\": [\n    \"GRCh38:1\",\n
+        \   \"GRCh38:chr1\",\n    \"GRCh38.p1:1\",\n    \"GRCh38.p1:chr1\",\n    \"GRCh38.p10:1\",\n
+        \   \"GRCh38.p10:chr1\",\n    \"GRCh38.p11:1\",\n    \"GRCh38.p11:chr1\",\n
+        \   \"GRCh38.p12:1\",\n    \"GRCh38.p12:chr1\",\n    \"GRCh38.p2:1\",\n    \"GRCh38.p2:chr1\",\n
+        \   \"GRCh38.p3:1\",\n    \"GRCh38.p3:chr1\",\n    \"GRCh38.p4:1\",\n    \"GRCh38.p4:chr1\",\n
+        \   \"GRCh38.p5:1\",\n    \"GRCh38.p5:chr1\",\n    \"GRCh38.p6:1\",\n    \"GRCh38.p6:chr1\",\n
+        \   \"GRCh38.p7:1\",\n    \"GRCh38.p7:chr1\",\n    \"GRCh38.p8:1\",\n    \"GRCh38.p8:chr1\",\n
+        \   \"GRCh38.p9:1\",\n    \"GRCh38.p9:chr1\",\n    \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n
+        \   \"NCBI:NC_000001.11\",\n    \"refseq:NC_000001.11\",\n    \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n
+        \   \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n    \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n
+        \   \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n    \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n
+        \ ],\n  \"alphabet\": \"ACGMNRT\",\n  \"length\": 248956422\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[gnomad-past-end-no-require-validation].yaml

@@ -0,0 +1,25 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-27T21:17:00Z\",\n  \"aliases\": [\n    \"GRCh38:1\",\n
+        \   \"GRCh38:chr1\",\n    \"GRCh38.p1:1\",\n    \"GRCh38.p1:chr1\",\n    \"GRCh38.p10:1\",\n
+        \   \"GRCh38.p10:chr1\",\n    \"GRCh38.p11:1\",\n    \"GRCh38.p11:chr1\",\n
+        \   \"GRCh38.p12:1\",\n    \"GRCh38.p12:chr1\",\n    \"GRCh38.p2:1\",\n    \"GRCh38.p2:chr1\",\n
+        \   \"GRCh38.p3:1\",\n    \"GRCh38.p3:chr1\",\n    \"GRCh38.p4:1\",\n    \"GRCh38.p4:chr1\",\n
+        \   \"GRCh38.p5:1\",\n    \"GRCh38.p5:chr1\",\n    \"GRCh38.p6:1\",\n    \"GRCh38.p6:chr1\",\n
+        \   \"GRCh38.p7:1\",\n    \"GRCh38.p7:chr1\",\n    \"GRCh38.p8:1\",\n    \"GRCh38.p8:chr1\",\n
+        \   \"GRCh38.p9:1\",\n    \"GRCh38.p9:chr1\",\n    \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n
+        \   \"NCBI:NC_000001.11\",\n    \"refseq:NC_000001.11\",\n    \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n
+        \   \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n    \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n
+        \   \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n    \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n
+        \ ],\n  \"alphabet\": \"ACGMNRT\",\n  \"length\": 248956422\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[gnomad-past-end].yaml

@@ -0,0 +1,25 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/GRCh38:1
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-27T21:17:00Z\",\n  \"aliases\": [\n    \"GRCh38:1\",\n
+        \   \"GRCh38:chr1\",\n    \"GRCh38.p1:1\",\n    \"GRCh38.p1:chr1\",\n    \"GRCh38.p10:1\",\n
+        \   \"GRCh38.p10:chr1\",\n    \"GRCh38.p11:1\",\n    \"GRCh38.p11:chr1\",\n
+        \   \"GRCh38.p12:1\",\n    \"GRCh38.p12:chr1\",\n    \"GRCh38.p2:1\",\n    \"GRCh38.p2:chr1\",\n
+        \   \"GRCh38.p3:1\",\n    \"GRCh38.p3:chr1\",\n    \"GRCh38.p4:1\",\n    \"GRCh38.p4:chr1\",\n
+        \   \"GRCh38.p5:1\",\n    \"GRCh38.p5:chr1\",\n    \"GRCh38.p6:1\",\n    \"GRCh38.p6:chr1\",\n
+        \   \"GRCh38.p7:1\",\n    \"GRCh38.p7:chr1\",\n    \"GRCh38.p8:1\",\n    \"GRCh38.p8:chr1\",\n
+        \   \"GRCh38.p9:1\",\n    \"GRCh38.p9:chr1\",\n    \"MD5:6aef897c3d6ff0c78aff06ac189178dd\",\n
+        \   \"NCBI:NC_000001.11\",\n    \"refseq:NC_000001.11\",\n    \"SEGUID:FCUd6VJ6uikS/VWLbhGdVmj2rOA\",\n
+        \   \"SHA1:14251de9527aba2912fd558b6e119d5668f6ace0\",\n    \"VMC:GS_Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n
+        \   \"sha512t24u:Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\",\n    \"ga4gh:SQ.Ya6Rs7DHhDeg7YaOSg1EoNi3U_nQ9SvO\"\n
+        \ ],\n  \"alphabet\": \"ACGMNRT\",\n  \"length\": 248956422\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1

tests/extras/cassettes/test_out_of_bounds[hgvs-n-insertion-past-end].yaml

@@ -0,0 +1,18 @@
+interactions:
+- request:
+    body: null
+    headers: {}
+    method: GET
+    uri: http://localhost:5000/seqrepo/1/metadata/refseq:NM_000551.3
+  response:
+    body:
+      string: "{\n  \"added\": \"2016-08-24T05:03:11Z\",\n  \"aliases\": [\n    \"MD5:215137b1973c1a5afcf86be7d999574a\",\n
+        \   \"NCBI:NM_000551.3\",\n    \"refseq:NM_000551.3\",\n    \"SEGUID:T12L0p2X5E8DbnL0+SwI4Wc1S6g\",\n
+        \   \"SHA1:4f5d8bd29d97e44f036e72f4f92c08e167354ba8\",\n    \"VMC:GS_v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n
+        \   \"sha512t24u:v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\",\n    \"ga4gh:SQ.v_QTc1p-MUYdgrRv4LMT6ByXIOsdw3C_\"\n
+        \ ],\n  \"alphabet\": \"ACGT\",\n  \"length\": 4560\n}\n"
+    headers: {}
+    status:
+      code: 200
+      message: OK
+version: 1