From ce870f4c300ff021b72097377b3c75c83d76477a Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Fri, 14 Aug 2026 05:33:01 +0200 Subject: [PATCH] Ask whether a download target can be written, not read A path a caller wants confluent to save something into was being run through the check meant for a file confluent is asked to read. That check forks, drops to the calling user and asks os.access for R_OK, which is false for every file that does not exist yet, so nodesupport servicedata and save_licenses could only be given a path that was already there. Handed a name to create, they refused, and refused in a way no caller was looking for, so the command printed nothing and exited zero. The previous commit worked around it by skipping the check for a download target, which fixed the symptom by removing the guard rather than by asking the right question. Ask the right question instead: whether the user could have created the file in that directory themselves. A path that is already a directory is a destination directory, anything else names the file, which is the same rule the code that goes on to write the file follows. So a caller can still only make confluent write where they could have written, and this now also catches an unwritable destination at the point the request is made rather than several layers further in. --- confluent_server/confluent/messages.py | 42 +++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/confluent_server/confluent/messages.py b/confluent_server/confluent/messages.py index 1d792d9e..01cb2ea7 100644 --- a/confluent_server/confluent/messages.py +++ b/confluent_server/confluent/messages.py @@ -592,18 +592,21 @@ def get_input_message(path, operation, inputdata, nodes=None, multinode=False, 'No known input handler for request') -def checkaccess(user, filename, pwent): - """Check if a user has read access to a file. +def checkaccess(user, filename, pwent, mode=os.R_OK): + """Check if a user has the given access to a path. - This function checks if the specified user has read access to the given - filename. It returns True if the user has read access, and False otherwise. + This function checks if the specified user has the access named by mode to + the given filename. It returns True if the user has it, False otherwise. + Use R_OK to ask whether the user could have read a file they want confluent + to send, and W_OK on a directory to ask whether they could have created a + file where they want confluent to save one. """ child = os.fork() if child == 0: os.setgroups(os.getgrouplist(user, pwent.pw_gid)) os.setgid(pwent.pw_gid) os.setuid(pwent.pw_uid) - if os.access(filename, os.R_OK): + if os.access(filename, mode): os._exit(0) os._exit(1) else: @@ -618,6 +621,18 @@ def isurl(value): return False return True if prefix else False + +def destdir(value): + """The directory a download will actually be written into. + + A path that is already a directory is a destination directory and the file + gets created inside it; anything else names the file itself. This is the + same rule the code that writes the file goes by. + """ + if os.path.isdir(value): + return value + return os.path.dirname(value) or '.' + class InputFirmwareUpdate(ConfluentMessage): urlsupported = False # Whether the named file is something confluent will read from the caller, @@ -652,11 +667,24 @@ class InputFirmwareUpdate(ConfluentMessage): if value.startswith('/var/log/confluent'): raise Exception( 'File transfer with /var/log/confluent is not supported') - if (curruser and not self.isdownload + if (curruser and not value.startswith('/var/lib/confluent/client_assets/')): try: pwent = pwd.getpwnam(curruser) - if not checkaccess(curruser, value, pwent): + if self.isdownload: + # Asking whether a destination can be read fails for + # every file that does not exist yet, which is most + # of them. The question that carries the same + # meaning here is whether the user could have + # created the file there themselves. + target = destdir(value) + if not checkaccess(curruser, target, pwent, + os.W_OK): + raise Exception( + '{0} is not writable by {1}, check the ' + 'directory and parent directory ownership ' + 'and permissions'.format(target, curruser)) + elif not checkaccess(curruser, value, pwent): errstr = '{0} is not readable by {1}, check the file and parent directory ownership and permissions'.format( value, curruser) raise Exception(errstr)