fix: get_tree returns 404 (not 403) for inaccessible trees — don't leak resource existence
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -392,9 +392,10 @@ async def get_tree(
|
|||||||
)
|
)
|
||||||
|
|
||||||
if not tree.is_active or not can_access_tree(current_user, tree):
|
if not tree.is_active or not can_access_tree(current_user, tree):
|
||||||
|
# Always 404, never 403. A 403 confirms the resource exists.
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
status_code=status.HTTP_404_NOT_FOUND,
|
||||||
detail="You don't have access to this tree"
|
detail="Tree not found"
|
||||||
)
|
)
|
||||||
|
|
||||||
return build_full_tree_response(tree)
|
return build_full_tree_response(tree)
|
||||||
|
|||||||
@@ -447,3 +447,55 @@ class TestVisibilityFilter:
|
|||||||
assert "author_name" in trees[0]
|
assert "author_name" in trees[0]
|
||||||
# visibility key should be present
|
# visibility key should be present
|
||||||
assert "visibility" in trees[0]
|
assert "visibility" in trees[0]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_tree_returns_404_not_403_for_other_account_tree(
|
||||||
|
self, client: AsyncClient, auth_headers: dict, test_db: AsyncSession
|
||||||
|
):
|
||||||
|
"""Account A must not learn that Account B's private tree exists."""
|
||||||
|
from app.models.tree import Tree
|
||||||
|
from app.models.account import Account
|
||||||
|
from app.models.user import User
|
||||||
|
from app.core.security import get_password_hash
|
||||||
|
import uuid
|
||||||
|
|
||||||
|
# Create a second account and user
|
||||||
|
account_b = Account(name="Other Corp", display_code="OTH00001")
|
||||||
|
test_db.add(account_b)
|
||||||
|
await test_db.flush()
|
||||||
|
|
||||||
|
user_b = User(
|
||||||
|
email=f"user-b-{uuid.uuid4().hex[:6]}@example.com",
|
||||||
|
name="User B",
|
||||||
|
password_hash=get_password_hash("TestPass123!"),
|
||||||
|
is_active=True,
|
||||||
|
account_id=account_b.id,
|
||||||
|
account_role="engineer",
|
||||||
|
)
|
||||||
|
test_db.add(user_b)
|
||||||
|
await test_db.flush()
|
||||||
|
|
||||||
|
# Create a private tree belonging to account_b
|
||||||
|
private_tree = Tree(
|
||||||
|
name="Secret Tree",
|
||||||
|
account_id=account_b.id,
|
||||||
|
author_id=user_b.id,
|
||||||
|
visibility="private",
|
||||||
|
tree_type="troubleshooting",
|
||||||
|
tree_structure={"id": "root", "type": "start", "children": []},
|
||||||
|
is_active=True,
|
||||||
|
is_default=False,
|
||||||
|
is_public=False,
|
||||||
|
status="published",
|
||||||
|
)
|
||||||
|
test_db.add(private_tree)
|
||||||
|
await test_db.commit()
|
||||||
|
|
||||||
|
response = await client.get(
|
||||||
|
f"/api/v1/trees/{private_tree.id}",
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert response.status_code == 404, (
|
||||||
|
f"Expected 404 but got {response.status_code} — "
|
||||||
|
"leaking tree existence to wrong tenant"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user