justinpakzad commented on PR #76106:
URL: https://github.com/apache/airflow/pull/76106#issuecomment-6560632080
> ### task state written through the public API is not Dag-author input
(`providers/snowflake/src/airflow/providers/snowflake/operators/snowpark_containers.py:256`)
> ```
> self.job_name = cast("str", external_id)
> ```
>
> The job name read back from `task_state_store` becomes `self.job_name`
here (and in `get_job_result`), and is then formatted unescaped into `DESCRIBE
SERVICE {...}`, `DROP SERVICE IF EXISTS {...}` and
`SYSTEM$GET_SERVICE_LOGS('{...}', ...)`. Task state can also be written through
the public REST API (`PUT .../taskStateStore`) with task-instance edit
permission, which is less than Dag authoring, so a stored value is not
necessarily something the operator wrote. Could the stored name be validated
against Snowflake identifier syntax before it reaches SQL (the names are
Snowflake-generated, so a strict check is safe), the `GET_SERVICE_LOGS`
argument be bound instead of formatted, and the DONE marker be tied to the job
name (e.g. `{job_name: "DONE"}`) so a marker set from outside cannot make the
operator skip the job either?
>
> ### Smaller observations
> * See inline comments on
`providers/snowflake/src/airflow/providers/snowflake/operators/snowpark_containers.py:143`.
Thanks for the second pass. All very good points and they have been
addressed in the latest push.
--
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]
