Skip to content

fix: use correct logical operator in resource ownership validation#11864

Open
yogeshwaran-c wants to merge 1 commit intoappwrite:1.9.xfrom
yogeshwaran-c:fix/resource-ownership-logical-operator
Open

fix: use correct logical operator in resource ownership validation#11864
yogeshwaran-c wants to merge 1 commit intoappwrite:1.9.xfrom
yogeshwaran-c:fix/resource-ownership-logical-operator

Conversation

@yogeshwaran-c
Copy link
Copy Markdown

Summary

  • Fixed incorrect use of && (AND) instead of || (OR) in resource ownership checks across three endpoints:
    • Functions/Http/Executions/Delete.php
    • Sites/Http/Logs/Get.php
    • Sites/Http/Logs/Delete.php
  • With &&, a request could pass the ownership check if only one of the two conditions (resourceType or resourceInternalId) was wrong, but the other was correct. The correct behavior is to reject if either condition fails.
  • The corresponding Functions/Http/Executions/Get.php endpoint already uses the correct || operator, confirming this was unintentional.

Test plan

  • Verify that attempting to delete an execution with a valid executionId but belonging to a different function correctly returns a 404
  • Verify that getting a site log with a valid logId but belonging to a different site correctly returns a 404
  • Verify that deleting a site log with a valid logId but belonging to a different site correctly returns a 404
  • Verify that normal get/delete operations on correctly-owned resources still work as expected

The resource ownership checks in execution delete and site log get/delete
endpoints used && (AND) instead of || (OR), which meant a resource could
pass the check if only one of the two conditions (resourceType or
resourceInternalId) was wrong. The correct behavior is to reject the
request if either condition fails, matching the pattern used in the
corresponding Get endpoint for executions.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 12, 2026

Greptile Summary

This PR fixes a security/correctness bug where && was used instead of || in resource ownership validation checks across three endpoints (Functions/Http/Executions/Delete.php, Sites/Http/Logs/Get.php, Sites/Http/Logs/Delete.php). With &&, a request could bypass ownership validation if only one of the two conditions (resourceType or resourceInternalId) was wrong — the correct || ensures a 404 is returned if either condition fails, consistent with Functions/Http/Executions/Get.php which already used the correct operator. A grep across src/ confirms no other instances of the && ownership-check pattern remain.

Confidence Score: 5/5

Safe to merge — the fix is minimal, clearly correct, and confirmed complete by a codebase-wide search.

All three changes are single-operator fixes that align with the existing reference implementation. No new logic introduced, no regressions possible, and no other instances of the pattern remain in the codebase.

No files require special attention.

Important Files Changed

Filename Overview
src/Appwrite/Platform/Modules/Functions/Http/Executions/Delete.php Fixes logical operator in ownership check from && to `
src/Appwrite/Platform/Modules/Sites/Http/Logs/Get.php Fixes logical operator in ownership check from && to `
src/Appwrite/Platform/Modules/Sites/Http/Logs/Delete.php Fixes logical operator in ownership check from && to `

Reviews (1): Last reviewed commit: "fix: use correct logical operator in res..." | Re-trigger Greptile

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.

1 participant