potiuk commented on code in PR #67857:
URL: https://github.com/apache/airflow/pull/67857#discussion_r3343134279
##########
providers/samba/src/airflow/providers/samba/transfers/gcs_to_samba.py:
##########
@@ -177,7 +177,17 @@ def _resolve_destination_path(self, source_object: str,
prefix: str | None = Non
source_object = os.path.relpath(source_object, start=prefix)
else:
source_object = os.path.basename(source_object)
- return os.path.join(self.destination_path, source_object)
+ # Source object names come from the GCS bucket and may contain ".."
segments.
+ # Normalize the joined path and make sure it stays within
destination_path so a
+ # crafted object name cannot resolve a write target outside the
configured directory.
+ resolved = os.path.normpath(os.path.join(self.destination_path,
source_object))
+ base = os.path.normpath(self.destination_path)
+ if resolved != base and not resolved.startswith(base + os.sep):
+ raise AirflowException(
Review Comment:
Good point. I went with `AirflowException` here for consistency — the same
method already raises it a few lines up (the wildcard check), so I matched the
existing style in the file rather than mixing two exception types in one method.
That said, we don't have an agreed way forward on the broader exception
migration yet (that discussion is still ongoing), so it's fine to just do it
this way. Latest push swaps the raise to `ValueError` and drops the
`known_airflow_exceptions.txt` bump accordingly.
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
