pierrejeambrun commented on code in PR #52795:
URL: https://github.com/apache/airflow/pull/52795#discussion_r2184773935
##########
airflow-core/src/airflow/ui/src/pages/Iframe.tsx:
##########
@@ -49,12 +49,38 @@ export const Iframe = ({ sandbox = "allow-same-origin
allow-forms" }: { readonly
return <ErrorPage />;
}
+ // Build the href URL with context parameters if the view has a destination
+ let src = iframeView.href;
+
+ if (iframeView.destination !== undefined && iframeView.destination !==
"nav") {
+ // Check if the href contains placeholders that need to be replaced
+ if (dagId !== undefined) {
+ src = src.replaceAll("{dag_id}", dagId);
+ }
+ if (runId !== undefined) {
+ src = src.replaceAll("{run_id}", runId);
+ }
+ if (taskId !== undefined) {
+ src = src.replaceAll("{task_id}", taskId);
Review Comment:
In the backend doc we have `{DAG_ID}, {RUN_ID}, etc...` as a pattern. Do we
want to replace it with lower case `{dag_id}, {run_id}, etc...`?
##########
airflow-core/src/airflow/ui/src/router.tsx:
##########
@@ -208,6 +257,15 @@ export const routerConfig = [
{ element: <TaskOverview />, index: true },
{ element: <TaskInstances />, path: "task_instances" },
{ element: <Events />, path: "events" },
+ {
+ // The following iframe sandbox setting is intentionally less
restrictive.
+ // This is considered safe because the framed content originates
from the Plugins,
+ // which is part of the deployment of Airflow and trusted as per
our security policy.
+ //
https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html
+ // They are not user provided plugins.
+ element: <Iframe sandbox="allow-scripts allow-same-origin
allow-forms" />,
+ path: "plugin/:page",
+ },
Review Comment:
Maybe extract this into a really small fn. "getPluginIframeRoute" or
something, in the same file.
Just so the comment, and how to construct the iframe component remains in
one place and not copy pasted everywhere. I'm scared of someone updating in one
place and not in other occurrences. (That doens't cost much to do and would
help preventing mistakes in the future)
--
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]