Fix: read-only guard should block non-datastore resources

[sagargg]

Summary

Align the datastore write guard with CKAN's actual datastore_create behavior. The previous implementation had the comparison inverted — it was blocking writes to datastore-managed resources and silently allowing them on upload / link ones, the exact opposite of CKAN's intent.

What the guard now does

Resource url_type	Without force	With force=True
datastore (managed by the datastore)	✅ allowed	✅ allowed
upload / link / any non-datastore value	❌ Validation Error (read-only)	✅ allowed
absent / no resource record (e.g. dict-form datastore_create)	✅ skipped	✅ allowed
any value, under AUTH_TYPE=jwt / anonymous	✅ skipped	✅ skipped

This matches CKAN's own check: if res_dict.get('url_type') != 'datastore' and not data_dict.get('force'): raise.

Code change

datastore/api/auth.py::ensure_resource_writable

• Flip the comparison: raise when url_type != "datastore" (the non-datastore case), not the other way around.
• Skip when url_type is absent — the dict-form of datastore_create authorises via package_id before the resource exists, so there's no record to protect.

Tests / fixture

• FakeCKAN seed (balancing_auction_results_2025) now carries url_type="datastore" so the existing write tests aren't accidentally tripping the guard.
• Guard tests in tests/auth/test_orchestration.py and the three endpoint test files (create / upsert / delete) are flipped: the "blocked without force" cases use url_type="upload"; new cases cover the datastore-managed (writable) and no-record (dict-create) skip paths.

Verification

• pytest — 330 passing under the CI-equivalent env.
• ruff check — clean.

Summary by CodeRabbit

Bug Fixes

• Updated resource write-protection logic for CKAN environments. Non-datastore resources now require a force flag to be written to, while datastore-managed resources remain writable without restrictions. Refined test coverage to reflect updated behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e4ffb79-bb21-4ff9-aee9-97b4058a09de

📥 Commits

Reviewing files that changed from the base of the PR and between 75e4838 and 3d046c6.

📒 Files selected for processing (6)

• datastore/api/auth.py
• tests/auth/test_orchestration.py
• tests/conftest.py
• tests/test_datastore_create.py
• tests/test_datastore_delete.py
• tests/test_datastore_upsert.py

---

📝 Walkthrough

Walkthrough

The PR inverts the CKAN read-only guard logic to block writes to non-datastore resources unless force=True. The implementation treats missing url_type as a guard skip condition, updates the docstring, and changes the guard condition from url_type == "datastore" to url_type != "datastore". Unit tests and E2E tests across create, delete, and upsert endpoints are updated to reflect the inverted behavior, with fixtures changed from url_type="datastore" to url_type="upload" for read-only resource scenarios.

Changes

CKAN Read-Only Guard Inversion

Layer / File(s)	Summary
Guard logic inversion datastore/api/auth.py	The ensure_resource_writable docstring clarifies that the CKAN guard skips when url_type is absent and blocks writes to resources where url_type is not "datastore" unless force=True. Implementation reads url_type into a local variable, returns early when missing, and inverts the blocking condition from previous equality logic to a != "datastore" check.
Guard unit test coverage tests/auth/test_orchestration.py	Unit tests validate that non-datastore resources (url_type="upload") fail authorization when force=False, succeed when force=True, datastore resources always succeed, the guard is skipped when url_type is absent or the resource record is empty, and non-CKAN auth types ("anonymous", "jwt") never trigger the guard.
Fixture baseline and E2E test validation tests/conftest.py, tests/test_datastore_create.py, tests/test_datastore_delete.py, tests/test_datastore_upsert.py	The fake_ckan fixture seeds balancing_auction_results_2025 with url_type="datastore" as the marker field. E2E tests across datastore_create, datastore_delete, and datastore_upsert endpoints update read-only resource fixtures from url_type="datastore" to url_type="upload" to align with the inverted guard condition, validating both the rejection without force and success with force=True.

🎯 3 (Moderate) | ⏱️ ~25 minutes

A guard stands reversed, wise code turns back,
what once was blocked now flows with stack,
datastore-marked resources leap free,
while others pause for force's key, 🐰✨
tests spring forth to verify the dance.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix: read-only guard should block non-datastore resources' directly and precisely describes the main behavioral change in the PR—inverting the write-protection logic to block non-datastore resources instead of datastore resources.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/datastore-write-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}