Add path access checker methods to prevent unauthorized access.

[jbernal0019]

1. Summary

This PR together with FNNDSC/pfcon#175 close a permission gap around ChRIS link files (.chrislink) by centralizing path-access authorization in core and re-enforcing it at plugin job runtime. A single source of truth for the access rule is introduced in core.models and reused at submission time (serializer validators), and at runtime (when links are followed and when output is registered).

Fix https://github.com/FNNDSC/engineering_man/issues/97.

2. Background and threat model

ChRIS link files act like filesystem symlinks — their path field can point at any storage path. Prior to this branch the access rule was only checked once, when a plugin instance was created, against the link file itself:

• Follow-time gap — a user could create a link file they own that points at another user's private data. At job execution the worker would dereference the link and read the target without re-checking authorization for the target path.
• Output-time gap — a plugin could emit a .chrislink file in its output directory pointing anywhere. Once registered, subsequent jobs would follow it.

The fix is to (a) make the access rule reusable as a single function, and (b) re-apply it every time a link is followed or a link file is about to be registered.

3. Changes

3.1 New core primitives in chris_backend/core/models.py

• user_can_access_obj(obj, user) — canonical read-access predicate. Returns True if the user owns the object, is the superuser chris, the object is public, or the user has any granted permission (directly or through a group). Works for ChrisFolder, ChrisFile, and ChrisLinkFile.
• PathAccessError — exception raised by the path validator. Its message is a human-readable reason suitable for surfacing to API clients.
• validate_path_access(user, path) — single source of truth for path-string access authorization. Enforces:
  • structural rules — ≥2 path parts; root ∈ {home, SERVICES, PIPELINES}; not a bare home/<user>; not home/<user>/feeds;
  • resolution — path must resolve to a ChrisFolder, ChrisFile, or ChrisLinkFile;
  • link-target restriction — a link whose target is PUBLIC or SHARED is rejected;
  • access rule — delegates to user_can_access_obj.
  • Returns the normalized path on success; raises PathAccessError on failure.

3.2 Runtime re-enforcement in chris_backend/plugininstances/services/pluginjobs.py

• New _check_path_access(linked_path) — delegates to core.models.validate_path_access, logs [CODE17] and raises ValueError on rejection, setting the plugin-instance error_code for diagnostics.
• Call site in find_all_storage_object_paths — whenever a .chrislink is encountered during input gathering, the link's target is re-checked. This closes the follow-time gap.
• Call site in _create_chris_link_file — link-file creation paths (e.g., unextpath parameters, ts plugins) re-check the resolved target before materializing a link.
• _register_output_files refuses remote .chrislink files — any link file that appears in the remote compute's output set is treated as untrusted, deleted from storage, and skipped. If deletion fails, the job is failed with [CODE07] and the exception is re-raised — a non-deletable attacker-controlled link in an output dir is a security-relevant state. Legit link files created by CUBE for unextpath/ts flows are registered directly as ChrisLinkFile objects and never enter this set, so they are unaffected.

3.3 Reuse / de-duplication

• chris_backend/plugininstances/serializers.py — validate_paths previously duplicated the entire structural + resolution + access logic inline. It now delegates to validate_path_access and translates PathAccessError into a DRF ValidationError. The user-visible error strings are preserved verbatim.
• chris_backend/filebrowser/services.py — get_folder_queryset previously inlined the access predicate. It now calls user_can_access_obj(folder, user). The unauthenticated branch (public-only) is untouched.

4. Files changed

File	Change
core/models.py	Adds user_can_access_obj, PathAccessError, validate_path_access, pathlib import.
core/tests/test_models.py	Adds UserCanAccessObjTests, ValidatePathAccessTests, ValidatePathAccessStorageTests (the storage one is @tag('integration')).
plugininstances/serializers.py	Slims validate_paths to delegate to validate_path_access.
plugininstances/services/pluginjobs.py	Adds _check_path_access, call sites in find_all_storage_object_paths and the link-creation flow, and the output-time .chrislink refusal block in _register_output_files.
plugininstances/tests/test_pluginjobs.py	Adds tests for the new behaviors (see §5).
plugininstances/models.py	Trims imports — PathAccessError/validate_path_access now live in core.models.
filebrowser/services.py	Uses user_can_access_obj in get_folder_queryset.

5. Tests

5.1 New tests

• UserCanAccessObjTests (unit) — owner / superuser / non-shared / public / explicit-permission cases.
• ValidatePathAccessTests (unit) — structural rejections with exact message strings, nonexistent path, folder owner/public/permission rule, superuser chris bypass.
• ValidatePathAccessStorageTests (@tag('integration')) — ChrisFile and ChrisLinkFile resolution branches and the PUBLIC/SHARED link-target rejection.
• test_pluginjobs.py — new cases:
  • _create_chris_link_file refuses unauthorized target (CODE17).
  • find_all_storage_object_paths refuses to follow a link with an unauthorized target; still follows authorized links.
  • _register_output_files refuses a remote .chrislink file and deletes it from storage while still registering regular outputs (integration).
  • _register_output_files fails the job with CODE07 when the link delete fails.
  • _register_output_files preserves legit CUBE-generated link files (integration).

5.2 How to run

just test --exclude-tag integration core.tests.test_models plugininstances.tests.test_pluginjobs plugininstances.tests.test_models plugininstances.tests.test_serializers
just test --tag integration core.tests.test_models plugininstances.tests.test_pluginjobs

[codacy-production]

Pull Request Overview

The PR aims to centralize path-access authorization and enforce strict validation across plugin job inputs and outputs to mitigate security risks associated with unauthorized link access. However, the current submission contains no code changes (empty diff), which prevents verification of the implementation or its security efficacy. \n\nOnce the implementation is provided, it must be validated against the specified acceptance criteria, particularly structural path rules, ChrisLinkFile target restrictions (rejecting PUBLIC/SHARED storage), and the correct propagation of error codes [CODE17] and [CODE07]. The absence of tests for these security-critical paths is a major gap.

About this PR

• The PR input contains no code changes (diff), making it impossible to verify the implementation or the existence of tests.
• No coverage report was provided to confirm that the new security-critical logic is actually exercised by tests.

Test suggestions

• user_can_access_obj correctly evaluates owner, superuser, public access, and granted permissions\n- [ ] validate_path_access enforces structural rules (min 2 parts, specific roots) and restricted path segments\n- [ ] validate_path_access rejects ChrisLinkFiles targeting PUBLIC or SHARED storage paths\n- [ ] find_all_storage_object_paths halts and sets [CODE17] when an unauthorized link is encountered during input gathering\n- [ ] _create_chris_link_file rejects unauthorized targets with [CODE17]\n- [ ] _register_output_files deletes remote .chrislink files and skips their registration while allowing valid outputs\n- [ ] _register_output_files fails the plugin instance with [CODE07] if a remote link file cannot be deleted from storage\n- [ ] PluginInstance serializers correctly translate PathAccessError into a DRF ValidationError

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. user_can_access_obj correctly evaluates owner, superuser, public access, and granted permissions\n- [ ] validate_path_access enforces structural rules (min 2 parts, specific roots) and restricted path segments\n- [ ] validate_path_access rejects ChrisLinkFiles targeting PUBLIC or SHARED storage paths\n- [ ] find_all_storage_object_paths halts and sets [CODE17] when an unauthorized link is encountered during input gathering\n- [ ] _create_chris_link_file rejects unauthorized targets with [CODE17]\n- [ ] _register_output_files deletes remote .chrislink files and skips their registration while allowing valid outputs\n- [ ] _register_output_files fails the plugin instance with [CODE07] if a remote link file cannot be deleted from storage\n- [ ] PluginInstance serializers correctly translate PathAccessError into a DRF ValidationError

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[rudolphpienaar]

Backend side looks good to me. This centralizes the path access rule and re-applies it at the relevant .chrislink follow/create/register points described in the PR summary.

My understanding of the fix is that .chrislink files in input paths remain valid, but their targets are now re-authorized at runtime before being followed. Plugin-produced .chrislink files in output are refused, which prevents untrusted plugin code from creating ChRIS-level links to arbitrary storage paths.

Important sidebar: I also reviewed the paired FNNDSC/pfcon#175. One non-blocking follow-up: in pfcon/storage/fslink_storage.py, FSLinkStorage.get_data() attempts to remove plugin-produced .chrislink files, but if os.remove(local_file_path) raises OSError, the exception is logged and processing continues. Since that can leave the link physically present in shared storage while still returning successful output metadata to CUBE, I think that failure should be fatal in a follow-up patch, matching this PR’s _register_output_files() behavior.

Approving this PR as implementing the backend portion of the fix.

[jbernal0019]

@rudolphpienaar Good call. Created issue FNNDSC/pfcon/issues/176 for the failed attempts to remove plugin-produced .chrislink files.