From 9b4d350b58e3c4fe76e2f51adab742340abd989e Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:02:47 +0200 Subject: [PATCH 1/7] Check artifact against result or run in controller Co-authored-by: Claude --- .../ibutsu_server/controllers/artifact_controller.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/backend/ibutsu_server/controllers/artifact_controller.py b/backend/ibutsu_server/controllers/artifact_controller.py index 68f8a52d..b0aaf31b 100644 --- a/backend/ibutsu_server/controllers/artifact_controller.py +++ b/backend/ibutsu_server/controllers/artifact_controller.py @@ -85,7 +85,9 @@ def download_artifact(id_, token_info=None, user=None): :rtype: file """ artifact, response = _build_artifact_response(id_) - if not project_has_user(artifact.result.project, user): + if (artifact.result and not project_has_user(artifact.result.project, user)) or ( + artifact.run and not project_has_user(artifact.run.project, user) + ): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN response.headers["Content-Disposition"] = f"attachment; filename={artifact.filename}" return response @@ -103,7 +105,9 @@ def get_artifact(id_, token_info=None, user=None): artifact = db.session.get(Artifact, id_) if not artifact: return HTTPStatus.NOT_FOUND.phrase, HTTPStatus.NOT_FOUND - if not project_has_user(artifact.result.project, user): + if (artifact.result and not project_has_user(artifact.result.project, user)) or ( + artifact.run and not project_has_user(artifact.run.project, user) + ): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN return artifact.to_dict() @@ -338,7 +342,9 @@ def delete_artifact(id_, token_info=None, user=None): artifact = db.session.get(Artifact, id_) if not artifact: return HTTPStatus.NOT_FOUND.phrase, HTTPStatus.NOT_FOUND - if not project_has_user(artifact.result.project, user): + if (artifact.result and not project_has_user(artifact.result.project, user)) or ( + artifact.run and not project_has_user(artifact.run.project, user) + ): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN session.delete(artifact) session.commit() From 4766f020b2c4b38ea1e861c806c7abc05ee0e249 Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:05:04 +0200 Subject: [PATCH 2/7] Fix type comparison in password recovery Co-authored-by: Claude --- backend/ibutsu_server/controllers/login_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/ibutsu_server/controllers/login_controller.py b/backend/ibutsu_server/controllers/login_controller.py index d68edf80..ea645484 100644 --- a/backend/ibutsu_server/controllers/login_controller.py +++ b/backend/ibutsu_server/controllers/login_controller.py @@ -291,7 +291,7 @@ def recover(body=None): if not user: return HTTPStatus.BAD_REQUEST.phrase, HTTPStatus.BAD_REQUEST # Create a random activation code. Base64 just for funsies - user.activation_code = urlsafe_b64encode(str(uuid4()).encode("utf8")).strip(b"=") + user.activation_code = urlsafe_b64encode(str(uuid4()).encode("utf8")).strip(b"=").decode() session.add(user) session.commit() return {}, HTTPStatus.CREATED From 8d0746f64c4ee9db0d78628ee63591e5e0d0f550 Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:08:58 +0200 Subject: [PATCH 3/7] Handle run/result with empty project Co-authored-by: Claude --- backend/ibutsu_server/util/projects.py | 2 ++ backend/tests/test_util.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/backend/ibutsu_server/util/projects.py b/backend/ibutsu_server/util/projects.py index 471daf42..d32c56fd 100644 --- a/backend/ibutsu_server/util/projects.py +++ b/backend/ibutsu_server/util/projects.py @@ -28,6 +28,8 @@ def project_has_user(project, user): return True if isinstance(project, str): project = get_project(project) + if project is None: + return False return user in project.users or project.owner.id == user.id diff --git a/backend/tests/test_util.py b/backend/tests/test_util.py index 18a39297..32458757 100644 --- a/backend/tests/test_util.py +++ b/backend/tests/test_util.py @@ -560,6 +560,12 @@ def test_project_has_user(make_project, make_user, user_role, expected_result): assert result is expected_result +def test_project_has_user_none_project(make_user): + """Test project_has_user returns False when project is None (data integrity issue).""" + user = make_user(email="user@test.com") + assert project_has_user(None, user) is False + + def test_project_has_user_with_string_ids(make_project, make_user): """Test project_has_user with string IDs instead of objects.""" owner = make_user(email="owner@test.com") From fa4b6d9d093f0e87fb94215b6aa2091b1b6a8c4c Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:14:40 +0200 Subject: [PATCH 4/7] Redundant result_id kwarg handling in controller Co-authored-by: Claude --- backend/ibutsu_server/controllers/artifact_controller.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/backend/ibutsu_server/controllers/artifact_controller.py b/backend/ibutsu_server/controllers/artifact_controller.py index b0aaf31b..41a40ba0 100644 --- a/backend/ibutsu_server/controllers/artifact_controller.py +++ b/backend/ibutsu_server/controllers/artifact_controller.py @@ -124,8 +124,6 @@ def get_artifact_list( """ query = db.select(Artifact) user = db.session.get(User, user) - if "result_id" in request.args: - result_id = request.args["result_id"] if result_id: query = query.where(Artifact.result_id == result_id) if run_id: From 190ccf9b06901684814e2e06086f7268c3284511 Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:28:03 +0200 Subject: [PATCH 5/7] function for checking artifact access Co-authored-by: Claude --- .../controllers/artifact_controller.py | 30 +++++++++++-------- backend/tests/test_util.py | 6 ++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/backend/ibutsu_server/controllers/artifact_controller.py b/backend/ibutsu_server/controllers/artifact_controller.py index 41a40ba0..324bee68 100644 --- a/backend/ibutsu_server/controllers/artifact_controller.py +++ b/backend/ibutsu_server/controllers/artifact_controller.py @@ -27,6 +27,20 @@ def __init__(self, message, status=HTTPStatus.BAD_REQUEST): self.status = status +def _user_has_artifact_access(artifact, user): + """Check whether the user has access to the artifact's project. + + Access is determined by the project associated with the artifact's result or run. + If the artifact is not linked to either a result or a run, access is denied + to avoid silently granting access to orphaned artifacts. + """ + if artifact.result: + return project_has_user(artifact.result.project, user) + if artifact.run: + return project_has_user(artifact.run.project, user) + return False + + def _get_form_param(form, *keys): """Get a form parameter supporting both camelCase and snake_case naming. @@ -68,9 +82,7 @@ def view_artifact(id_, token_info=None, user=None): :rtype: file """ artifact, response = _build_artifact_response(id_) - if (artifact.result and not project_has_user(artifact.result.project, user)) or ( - artifact.run and not project_has_user(artifact.run.project, user) - ): + if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN return response @@ -85,9 +97,7 @@ def download_artifact(id_, token_info=None, user=None): :rtype: file """ artifact, response = _build_artifact_response(id_) - if (artifact.result and not project_has_user(artifact.result.project, user)) or ( - artifact.run and not project_has_user(artifact.run.project, user) - ): + if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN response.headers["Content-Disposition"] = f"attachment; filename={artifact.filename}" return response @@ -105,9 +115,7 @@ def get_artifact(id_, token_info=None, user=None): artifact = db.session.get(Artifact, id_) if not artifact: return HTTPStatus.NOT_FOUND.phrase, HTTPStatus.NOT_FOUND - if (artifact.result and not project_has_user(artifact.result.project, user)) or ( - artifact.run and not project_has_user(artifact.run.project, user) - ): + if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN return artifact.to_dict() @@ -340,9 +348,7 @@ def delete_artifact(id_, token_info=None, user=None): artifact = db.session.get(Artifact, id_) if not artifact: return HTTPStatus.NOT_FOUND.phrase, HTTPStatus.NOT_FOUND - if (artifact.result and not project_has_user(artifact.result.project, user)) or ( - artifact.run and not project_has_user(artifact.run.project, user) - ): + if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN session.delete(artifact) session.commit() diff --git a/backend/tests/test_util.py b/backend/tests/test_util.py index 32458757..1069343a 100644 --- a/backend/tests/test_util.py +++ b/backend/tests/test_util.py @@ -566,6 +566,12 @@ def test_project_has_user_none_project(make_user): assert project_has_user(None, user) is False +def test_project_has_user_invalid_string_project_id(make_user): + """Test project_has_user returns False when a string project ID resolves to None.""" + user = make_user(email="user@test.com") + assert project_has_user("non-existent-project-id", user) is False + + def test_project_has_user_with_string_ids(make_project, make_user): """Test project_has_user with string IDs instead of objects.""" owner = make_user(email="owner@test.com") From c3ace6830ea35a0baa56b3ea60318e62c428bcb5 Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:32:47 +0200 Subject: [PATCH 6/7] pass the project object when checking user Co-authored-by: Claude --- backend/ibutsu_server/controllers/import_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/ibutsu_server/controllers/import_controller.py b/backend/ibutsu_server/controllers/import_controller.py index deea7135..606dd4b4 100644 --- a/backend/ibutsu_server/controllers/import_controller.py +++ b/backend/ibutsu_server/controllers/import_controller.py @@ -102,7 +102,7 @@ def add_import( project_obj = get_project(project) if not project_obj: return f"Project {project} doesn't exist", HTTPStatus.BAD_REQUEST - if not project_has_user(project, user): + if not project_has_user(project_obj, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN data["project_id"] = project_obj.id if request.form.get("metadata"): From 3f4a3ae042d3b5b59045c83d7422dd1269f887d1 Mon Sep 17 00:00:00 2001 From: mshriver Date: Tue, 14 Apr 2026 14:33:41 +0200 Subject: [PATCH 7/7] Check that an artifact is found Co-authored-by: Claude --- backend/ibutsu_server/controllers/artifact_controller.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/ibutsu_server/controllers/artifact_controller.py b/backend/ibutsu_server/controllers/artifact_controller.py index 324bee68..2e47d0bd 100644 --- a/backend/ibutsu_server/controllers/artifact_controller.py +++ b/backend/ibutsu_server/controllers/artifact_controller.py @@ -82,6 +82,8 @@ def view_artifact(id_, token_info=None, user=None): :rtype: file """ artifact, response = _build_artifact_response(id_) + if not isinstance(artifact, Artifact): + return artifact, response if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN return response @@ -97,6 +99,8 @@ def download_artifact(id_, token_info=None, user=None): :rtype: file """ artifact, response = _build_artifact_response(id_) + if not isinstance(artifact, Artifact): + return artifact, response if not _user_has_artifact_access(artifact, user): return HTTPStatus.FORBIDDEN.phrase, HTTPStatus.FORBIDDEN response.headers["Content-Disposition"] = f"attachment; filename={artifact.filename}"