Skip to content

Validate and escape what manage-datasets/ writes - #2525

Open
jh-RLI wants to merge 3 commits into
developfrom
fix-2508-manage-datasets-validation
Open

jh-RLI wants to merge 3 commits into
developfrom
fix-2508-manage-datasets-validation

Conversation

@jh-RLI

@jh-RLI jh-RLI commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Closes #2508. Fixes part 4 of #2509 (the injection); the rest of #2509 is deliberately left open.

#2508: the list validation never ran

DRF calls validate_<field name>. The field is datasets, and the hook was validate_dataset. So an empty list and duplicate names were accepted. The hook is now renamed to validate_datasets.

#2509 part 4: SPARQL injection

add_datasets_to_scenario built its INSERT DATA with an f-string, pasting two user-controlled values between quotes:

  • the label, which is a table's human-readable title and editable by the table's owner;
  • the external address, which is only checked to start with the databus host.

Any logged-in user can create a bundle, and so own one. A quote in either value closed the literal and turned the rest into triples in the OEKG. The new tests showed it on develop: a crafted label wrote 7 triples instead of 5, and the two extra were the payload's own.

The two query builders were first extracted unchanged (insert_dataset_query, dataset_exists_query), so the tests can run the statement text against an in-memory rdflib graph with no Fuseki needed. Both values are now written as Literal(...).n3(). The identifiers are server-minted uuids, and the OEO terms are constants.

Deliberately not in this PR

The rest of #2509 changes what the route writes, so it needs WF-06's retire-or-fix decision, plus a count of the affected triples on production:

  • the predicates (RO_0002233/4 vs OEO_00020437/6);
  • has_iri/has_key vs the closed shape;
  • the missing uuid.

Tests

8 new tests in api/tests/test_manage_datasets.py: 3 for the list rules, 5 for the escaping (quote in the label, quote in the address, newline and backslash, the ASK, a plain link).

python manage.py test api oekg factsheet: 407 tests, green locally (105 skipped: no Fuseki). One subprocess test needed a rerun under the default settings, because of the isolated test DB.

🤖 Generated with Claude Code

jh-RLI and others added 2 commits September 23, 2026 20:04
Two defects on the legacy route that attaches datasets to a scenario:

- The list hook was named validate_dataset, but DRF calls
  validate_<field> and the field is `datasets`, so the empty-list and
  duplicate-name rules never ran (#2508). Renamed.
- insert_dataset_query and dataset_exists_query (extracted unchanged
  from their callers first, so the tests could run the statement text
  against an in-memory rdflib graph) put the label and the address
  between quotes. The label is a table's title, editable by its owner;
  an external address is checked only for its prefix. The tests showed
  a quote in either writing 7 triples instead of 5, the extra two being
  the payload's own. Both are now Literal(...).n3().

The predicates and closed-shape properties of #2509 are unchanged on
purpose: fixing them changes the data representation and needs the
retire-or-fix decision plus a production count first.

Closes #2508
Refs #2509

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jh-RLI jh-RLI self-assigned this Sep 24, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScenarioBundleScenarioDatasetSerializer.validate_dataset is never called — dataset validation is dead code

1 participant