Skip to content

Commit 69e3779

Browse files
committed
Correct the /config path validation comment
The comment claimed set_g_cred_path()'s validation kept the endpoint from doubling as a probe for arbitrary filesystem paths. That is backwards: the validation constrains the file's kind (a readable .json) but not its location, so the valid/invalid response is an existence oracle for readable JSON files anywhere on disk. Record what is actually true, why the path is left unconstrained (service account credentials normally live outside --data-root), and why the oracle is accepted rather than fixed (localhost-default bind plus XSRF). CodeQL alert #15 (py/path-injection) is dismissed on the same rationale.
1 parent 7785dea commit 69e3779

1 file changed

Lines changed: 13 additions & 4 deletions

File tree

vizseq/server.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,19 @@ def get(self):
287287

288288
def post(self):
289289
g_cred_path = self.get_argument('g_cred_path', '')
290-
# set_g_cred_path() accepts only a readable JSON file, which keeps this
291-
# endpoint from doubling as a probe for arbitrary filesystem paths. It
292-
# also applies the credentials to this process, so /g_translate picks
293-
# them up without a restart.
290+
# The path is deliberately unconstrained: service account credentials
291+
# normally live outside --data-root (e.g. ~/.config/gcloud), so
292+
# confining it to an allowlisted root would break the common setup.
293+
# set_g_cred_path() therefore validates the file's kind (a readable
294+
# .json) but not its location, which does make the valid/invalid
295+
# response an existence oracle for readable JSON files anywhere on
296+
# disk. That is accepted rather than fixed: the server binds to
297+
# localhost by default and requires an XSRF token, so reaching this
298+
# needs an operator to expose it deliberately. See CodeQL alert #15
299+
# (py/path-injection), dismissed on the same rationale. Revisit if
300+
# this endpoint ever becomes reachable without those two conditions.
301+
# set_g_cred_path() also applies the credentials to this process, so
302+
# /g_translate picks them up without a restart.
294303
try:
295304
set_g_cred_path(g_cred_path)
296305
except (OSError, ValueError):

0 commit comments

Comments
 (0)