fix(logs): compare whole segments in path guard

[MaxFreedomPollard]

Description

The log viewer treats a directory that merely shares a name prefix with an allowed root as though it sat inside that root. With the roots shipped at backend/app/services/log_service.py:30-36, both /opt-private/creds.txt and /homestead/.ssh/id_rsa resolve as allowed log paths, on the strength of /opt and /home.

The cause is backend/app/services/log_service.py:47. is_path_allowed resolves the request with os.path.realpath and then tests real_path.startswith(allowed_dir). That compares text, not path segments, so it cannot tell /opt/app.log from /opt-private/creds.txt. The comment that introduces the list at log_service.py:29 calls it "Allowed directories for log file access (path traversal protection)", which is not what the loop underneath it implements. Line numbers throughout are as of dev at d866de5.

FileService is the exact sibling here rather than a distant analogy. It guards nearly the same roots (backend/app/services/file_service.py:45) and returns a denial string identical to LogService's, character for character (file_service.py:123 against log_service.py:81). It also gets the comparison right, at file_service.py:96-100: real_path == root or real_path.startswith(root + os.sep). The same segment-wise form appears at backend/app/api/deploy.py:197, backend/app/services/upload_service.py:66, backend/app/services/application_lifecycle_service.py:49, backend/app/services/manifest_apply_service.py:941 and backend/app/services/linked_panel_agent.py:251. LogService does not.

One qualification on reach, so this is not read as more than it is. Every route in backend/app/api/logs.py is @admin_required (lines 9, 17, 32, 47, 58, 71 and 85), so this is not a privilege escalation and I am not presenting it as one. The guard is still worth correcting on its own terms: it also gates truncation at log_service.py:367, where clear_log runs truncate -s 0 against whatever it admits, and streaming at log_service.py:409. A path that lands in a prefix sibling is currently accepted by all four doors, including the destructive one.

The change is one line and a comment naming the case. One consequence worth naming: a SERVERKIT_DIR set with a trailing slash no longer matches its own root, since realpath never returns one and allowed_dir + os.sep becomes a doubled separator; that fails closed, and it is exactly how file_service.py:99-100 already treats the same value. LogService has no PROTECTED_ROOTS exclusion of the kind FileService grew for GHSA-rm3m-9mvw-68fh; adding one is a separate logical change and this patch does not attempt it.

Related Issues

None.

Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)
• 🚀 New feature (non-breaking change which adds functionality)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected)
• 📝 Documentation update
• 🎨 UI/UX improvement

How Has This Been Tested?

The fix ships with backend/tests/test_log_path_containment.py, four tests in the shape of test_files_readonly_roots.py: a monkeypatched tmp_path root with a prefix sibling beside it, one assertion against the literal shipped constants, and a no-regression case. The shipped-constant case asserts /opt rather than /var/log, because realpath('/var/log') is /private/var/log on a macOS dev box and a /var/* assertion would test the developer's filesystem instead of the guard.

The test earns its place. On an unmodified dev checkout at d866de5 it gives 3 failed, 1 passed. test_shipped_roots_reject_a_prefix_sibling fails on assert not True, where True = is_path_allowed('/opt-private'). test_clear_log_refuses_a_prefix_sibling fails on assert 'Log file not found' == 'Access denied: path not in allowed directories', which is the useful one: the broken guard admits the path and falls through to the existence check below it, so the guard itself is demonstrably what lets the path in. That target deliberately names a file that does not exist, so even the failing run stops at the existence check and never reaches truncate.

With the fix applied, ./venv/bin/python -m pytest tests/test_log_path_containment.py -q gives 4 passed in 0.34s.

Every other test file that touches LogService or the sibling guard, run together as ./venv/bin/python -m pytest tests/test_log_path_containment.py tests/test_files_rbac.py tests/test_app_observability_runtime.py tests/test_connect_logs.py tests/test_files_readonly_roots.py -q, gives 121 passed.

The new file is collected by CI shard 3, so I ran that shard exactly as the workflow does: python -m pytest tests --splits 4 --group 3 -q gives 1 failed, 1311 passed, 35 skipped, 3948 deselected in 134.15s. The single failure is tests/test_probe_honesty_metrics.py::test_one_failed_section_does_not_blank_the_others, and it is not mine: it reproduces identically on an unmodified dev checkout at d866de5 with a clean tree, failing on isinstance(metrics['cpu'], dict) because the CPU probe returns None on this machine. The other three shards carry four more failures that also reproduce identically on unmodified dev: tests/test_api_surface_inventory.py::test_committed_surface_matches_served_surface on shard 1, which reports new operations: ['GET /connect/managed-profile'] because the dev head added that route without regenerating docs/API_SURFACE.md, and will be red on CI for any branch off that head; tests/test_backup_protection.py::test_smart_backup_files_incremental_roundtrip and ::test_full_backup_resets_snar on shard 1, because bsdtar on macOS has no --listed-incremental; and tests/test_fleet_doctor.py::test_dns_fail_when_unresolved on shard 2.

The ratchet scripts in backend/tests/ all stay inside their ceilings: check_test_count.py reports collected=5295 baseline=5279, error shape 1147 of 1150, JWT-only 608 of 608, identity door 69 of 69, raw subprocess 24 of 24, stub adoption 26 of 26, status sniffing 0 of 0, unreported crash 0 of 0. No floor bump was needed.

Because the change touches backend/app, I ran the Bandit gate as dev/dev.sh does, bandit -r backend/app --ini backend/.bandit --severity-level medium. It reports 2 low, 13 medium and 0 high, which is the same result the unmodified dev checkout produces, and none of the findings are in log_service.py.

All runs were on macOS (Darwin 25.2.0) with Python 3.11.15 against backend/venv. No route was added or removed, so docs/API_SURFACE.md is unchanged.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added a concise user-facing note under Unreleased in CHANGELOG.md, or explained why this change needs no release note
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes