korbit-ai[bot] commented on code in PR #32656: URL: https://github.com/apache/superset/pull/32656#discussion_r1994027766
########## scripts/build_docker.py: ########## @@ -0,0 +1,294 @@ +#!/usr/bin/env python3 + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import os +import re +import subprocess +from textwrap import dedent + +import click + +REPO = "viveksingh27/bi-superset" +CACHE_REPO = f"{REPO}-cache" +BASE_PY_IMAGE = "3.10-slim-bookworm" + + +def run_cmd(command: str, raise_on_failure: bool = True) -> str: + process = subprocess.Popen( + command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True + ) + + output = "" + if process.stdout is not None: + for line in iter(process.stdout.readline, ""): + print(line.strip()) # Print the line to stdout in real-time + output += line + + process.wait() # Wait for the subprocess to finish + + if process.returncode != 0 and raise_on_failure: + raise subprocess.CalledProcessError(process.returncode, command, output) + return output + + +def get_git_sha() -> str: + return run_cmd("git rev-parse HEAD").strip() + + +def get_build_context_ref(build_context: str) -> str: + """ + Given a context, return a ref: + - if context is pull_request, return the PR's id + - if context is push, return the branch + - if context is release, return the release ref + """ + + event = os.getenv("GITHUB_EVENT_NAME") + github_ref = os.getenv("GITHUB_REF", "") + + if event == "pull_request": + github_head_ref = os.getenv("GITHUB_HEAD_REF", "") + return re.sub("[^a-zA-Z0-9]", "-", github_head_ref)[:40] + elif event == "release": + return re.sub("refs/tags/", "", github_ref)[:40] + elif event == "push": + return re.sub("[^a-zA-Z0-9]", "-", re.sub("refs/heads/", "", github_ref))[:40] + return "" + + +def is_latest_release(release: str) -> bool: + output = ( + run_cmd( + f"./scripts/tag_latest_release.sh {release} --dry-run", + raise_on_failure=False, + ) + or "" + ) + return "SKIP_TAG::false" in output + + +def make_docker_tag(l: list[str]) -> str: # noqa: E741 + return f"{REPO}:" + "-".join([o for o in l if o]) + + +def get_docker_tags( + build_preset: str, + build_platforms: list[str], + sha: str, + build_context: str, + build_context_ref: str, + force_latest: bool = False, +) -> set[str]: + """ + Return a set of tags given a given build context + """ + tags: set[str] = set() + tag_chunks: list[str] = [] + + is_latest = is_latest_release(build_context_ref) + + if build_preset != "lean": + # Always add the preset_build name if different from default (lean) + tag_chunks += [build_preset] + + if len(build_platforms) == 1: + build_platform = build_platforms[0] + short_build_platform = build_platform.replace("linux/", "").replace("64", "") + if short_build_platform != "amd": + # Always a platform indicator if different from default (amd) + tag_chunks += [short_build_platform] + + # Always craft a tag for the SHA + tags.add(make_docker_tag([sha] + tag_chunks)) + # also a short SHA, cause it's nice + tags.add(make_docker_tag([sha[:7]] + tag_chunks)) + + if build_context == "release": + # add a release tag + tags.add(make_docker_tag([build_context_ref] + tag_chunks)) + if is_latest or force_latest: + # add a latest tag + tags.add(make_docker_tag(["latest"] + tag_chunks)) + elif build_context == "push" and build_context_ref == "master": + tags.add(make_docker_tag(["master"] + tag_chunks)) + elif build_context == "pull_request": + tags.add(make_docker_tag([f"pr-{build_context_ref}"] + tag_chunks)) + return tags + + +def get_docker_command( + build_preset: str, + build_platforms: list[str], + is_authenticated: bool, + sha: str, + build_context: str, + build_context_ref: str, + force_latest: bool = False, +) -> str: + tag = "" # noqa: F841 + build_target = "" + py_ver = BASE_PY_IMAGE + docker_context = "." + + if build_preset == "dev": + build_target = "dev" + elif build_preset == "lean": + build_target = "lean" + elif build_preset == "py311": + build_target = "lean" + py_ver = "3.11-slim-bookworm" + elif build_preset == "websocket": + build_target = "" + docker_context = "superset-websocket" + elif build_preset == "ci": + build_target = "ci" + elif build_preset == "dockerize": + build_target = "" + docker_context = "-f dockerize.Dockerfile ." + else: + print(f"Invalid build preset: {build_preset}") + exit(1) + + # Try to get context reference if missing + if not build_context_ref: + build_context_ref = get_build_context_ref(build_context) + + tags = get_docker_tags( + build_preset, + build_platforms, + sha, + build_context, + build_context_ref, + force_latest, + ) + docker_tags = ("\\\n" + 8 * " ").join([f"-t {s} " for s in tags]) + + docker_args = "--load" if not is_authenticated else "--push" + target_argument = f"--target {build_target}" if build_target else "" + + cache_ref = f"{CACHE_REPO}:{py_ver}" + if len(build_platforms) == 1: + build_platform = build_platforms[0] + short_build_platform = build_platform.replace("linux/", "").replace("64", "") + cache_ref = f"{CACHE_REPO}:{py_ver}-{short_build_platform}" + platform_arg = "--platform " + ",".join(build_platforms) + + cache_from_arg = f"--cache-from=type=registry,ref={cache_ref}" + cache_to_arg = ( + f"--cache-to=type=registry,mode=max,ref={cache_ref}" if is_authenticated else "" + ) + build_arg = f"--build-arg PY_VER={py_ver}" if py_ver else "" + actor = os.getenv("GITHUB_ACTOR") + + return dedent( + f"""\ + docker buildx build \\ + {docker_args} \\ + {docker_tags} \\ + {cache_from_arg} \\ + {cache_to_arg} \\ + {build_arg} \\ + {platform_arg} \\ + {target_argument} \\ + --label sha={sha} \\ + --label target={build_target} \\ + --label build_trigger={build_context} \\ + --label base={py_ver} \\ + --label build_actor={actor} \\ + {docker_context}""" + ) + + [email protected]() [email protected]( + "build_preset", + type=click.Choice(["lean", "dev", "dockerize", "websocket", "py311", "ci"]), +) [email protected]("build_context", type=click.Choice(["push", "pull_request", "release"])) [email protected]( + "--platform", + type=click.Choice(["linux/arm64", "linux/amd64"]), + default=["linux/amd64"], + multiple=True, +) [email protected]("--build_context_ref", help="a reference to the pr, release or branch") [email protected]("--dry-run", is_flag=True, help="Run the command in dry-run mode.") [email protected]("--verbose", is_flag=True, help="Print more info") [email protected]( + "--force-latest", is_flag=True, help="Force the 'latest' tag on the release" +) +def main( + build_preset: str, + build_context: str, + build_context_ref: str, + platform: list[str], + dry_run: bool, + force_latest: bool, + verbose: bool, +) -> None: + """ + This script executes docker build and push commands based on given arguments. + """ + + is_authenticated = ( + True if os.getenv("DOCKERHUB_TOKEN") and os.getenv("DOCKERHUB_USER") else False + ) + + if force_latest and build_context != "release": + print( + "--force-latest can only be applied if the build context is set to 'release'" + ) + exit(1) + + if build_context == "release" and not build_context_ref.strip(): + print("Release number has to be provided") + exit(1) + + docker_build_command = get_docker_command( + build_preset, + platform, + is_authenticated, + get_git_sha(), + build_context, + build_context_ref, + force_latest, + ) + + if not dry_run: + print("Executing Docker Build Command:") + print(docker_build_command) + script = "" + if os.getenv("DOCKERHUB_USER"): + script = dedent( + f"""\ + docker logout + docker login --username "{os.getenv("DOCKERHUB_USER")}" --password "{os.getenv("DOCKERHUB_TOKEN")}" Review Comment: ### Exposed Docker Credentials in Command Line <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Docker credentials are being passed directly as command line arguments, which can expose them in process listings and command history. ###### Why this matters Command line arguments can be viewed by other users on the system via ps, history, or similar commands, potentially exposing sensitive credentials. ###### Suggested change ∙ *Feature Preview* Use stdin or docker config for credential input: ```python run_cmd(f"echo $DOCKERHUB_TOKEN | docker login --username $DOCKERHUB_USER --password-stdin") ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2a1afc5e-708b-4993-b300-dffbf8d4fe0e?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:cda1ffdd-b056-42c0-b569-0592642b40c8 --> ########## superset-frontend/plugins/plugin-chart-handlebars/src/components/Handlebars/HandlebarsViewer.tsx: ########## @@ -76,28 +96,159 @@ export const HandlebarsViewer = ({ }; // usage: {{dateFormat my_date format="MMMM YYYY"}} -Handlebars.registerHelper('dateFormat', function (context, block) { +hb.registerHelper('dateFormat', function (context: any, block: any) { const f = block.hash.format || 'YYYY-MM-DD'; return moment(context).format(f); }); +hb.registerHelper('parseJSON', (jsonString: string) => { + if (jsonString === undefined) + throw Error('Please call with an object. Example: `parseJSON myObj`'); + return JSON.parse(jsonString); +}); + +hb.registerHelper('inc', (value: string) => { + if (value === undefined) + throw Error('Please call with an object. Example: `inc @index`'); + return parseInt(value, 10) + 1; +}); + // usage: {{ }} -Handlebars.registerHelper('stringify', (obj: any, obj2: any) => { +hb.registerHelper('stringify', (obj: any, obj2: any) => { // calling without an argument if (obj2 === undefined) throw Error('Please call with an object. Example: `stringify myObj`'); return isPlainObject(obj) ? JSON.stringify(obj) : String(obj); }); -Handlebars.registerHelper( - 'formatNumber', - function (number: any, locale = 'en-US') { - if (typeof number !== 'number') { - return number; +hb.registerHelper('formatNumber', function (number: any, locale = 'en-US') { + if (typeof number !== 'number') { + return number; + } + return number.toLocaleString(locale); +}); + +HandlebarsGroupBy.register(hb); + +// Example async function to fetch presigned URLs +async function fetchPresignedUrls0(imageLinks: string[]): Promise<string[]> { + const response = await fetch('/api/v1/presigned_urls/', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ image_links: imageLinks }), + }); + + if (response.ok) { + const data = await response.json(); + return data.presigned_urls; + } + console.error('Error fetching presigned URLs'); + return []; +} + +// Register async helper +hb.registerHelper('getPresignedUrls', async (imageLinks: string[]) => { + const urls = await fetchPresignedUrls0(imageLinks); + const urls1 = JSON.parse(urls as unknown as string); + const presignedUrls = urls1.map( + (item: { presigned_url: string }) => item.presigned_url, + ); + return presignedUrls; +}); + +hb.registerHelper('jsonToHtmlTable', (jsonData1: string, jsonData2: string) => { + const assignmentData = JSON.parse(jsonData1); + const reviewData = JSON.parse(jsonData2); + const items = assignmentData.items || []; + const errors = reviewData.diff || []; + let html = ''; + + let Total = assignmentData.total || ''; + const Date = assignmentData.txn ? assignmentData.txn.date || '' : ''; + const Hour = assignmentData.txn ? assignmentData.txn.hour || '' : ''; + const Minute = assignmentData.txn ? assignmentData.txn.minute || '' : ''; + const Suffix = assignmentData.txn ? assignmentData.txn.suffix || '' : ''; + let Channel = assignmentData.channel || ''; + + if (items) { + errors.forEach((error: any[][]) => { + if (error[1][0] === 'total') { + Total += `<br><span class="is_error">${error[2][0]}</span>`; // Add the incorrect original quantity + } else if (error[1][0] === 'channel') { + Channel += `<br><span class="is_error">${error[2][0]}</span>`; // Add the incorrect original quantity + } + }); + + html += + '<h3>Summary</h3><table id="receipt_summary_table" class="pretty_table"><tbody>'; + html += `<tr><td>Total</td><td>${Total}</td></tr> + <tr><td>Transaction Date</td><td>${Date}</td></tr> + <tr><td>Transaction Time</td><td>${Hour}:${Minute}:${Suffix}</td></tr> + <tr><td>Channel</td><td>${Channel}</td></tr>`; + html += '</tbody></table>'; + // Check if items exist + if (items.length === 0) { + html += '<h3>Items</h3><br><p>No items found in the receipt data.</p>'; + return html; } - return number.toLocaleString(locale); - }, -); -Helpers.registerHelpers(Handlebars); -HandlebarsGroupBy.register(Handlebars); + if (items) { + html += + '<h3>Items</h3><table id="receipt_item_table" class="pretty_table"><tbody>'; + items.forEach((item: any, index: any) => { + let { qty } = item; + let rin = item.upc || item.rin || ''; + let rsd = item.rsd || ''; + let { amount } = item; + const errorMessages: string[] = []; + + // Check for quantity errors in the review data + errors.forEach((error: any[][]) => { + if (error[1][1] === index && error[1][2] === 'qty') { + errorMessages.push('Incorrect Quantity'); + qty += `<br><span class="is_error">${error[2][0]}</span>`; // Add the incorrect original quantity + } else if ( + error[1][1] === index && + (error[1][2] === 'rin' || error[1][2] === 'upc') + ) { + rin += `<br><span class="is_error">${error[2][0]}</span>`; + errorMessages.push('Incorrect RIN'); + } else if (error[1][1] === index && error[1][2] === 'rsd') { + rsd += `<br><span class="is_error">${error[2][0]}</span>`; + errorMessages.push('Incorrect RSD'); + } else if (error[1][1] === index && error[1][2] === 'amount') { + amount += `<br><span class="is_error">${error[2][0]}</span>`; + errorMessages.push('Incorrect amount'); + } + }); + + const perItem = ( + parseFloat(item.amount) / parseInt(item.qty, 10) + ).toFixed(2); + + html += `<tr id="item_${index}" class=""> + <td>${item.type}</td> + <td>${qty}</td> + <td>${rin}</td> + <td>${rsd}</td> + <td>${amount}</td> + <td>${perItem}</td> + <td>${errorMessages.join(', ')}</td> + </tr>`; Review Comment: ### Unescaped HTML Interpolation <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Direct interpolation of potentially untrusted data into HTML string without proper escaping could lead to XSS vulnerabilities. ###### Why this matters If any of these fields contain malicious HTML/JavaScript, it would be executed when rendered in the browser. ###### Suggested change ∙ *Feature Preview* ```typescript function escapeHtml(unsafe: string) { return unsafe .replace(/&/g, "&") .replace(/</g, "<") .replace(/>/g, ">") .replace(/"/g, """) .replace(/'/g, "'"); } html += `<tr id="item_${escapeHtml(String(index))}" class=""> <td>${escapeHtml(item.type)}</td> <td>${escapeHtml(String(qty))}</td> // ... escape other fields similarly </tr>`; ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/427c7f0a-3e37-4a7b-8961-a6d6db5a6726?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:0ee7bf6a-992b-44ad-a27a-df74aa61e5f9 --> ########## docker/pythonpath_dev/superset_config.py: ########## @@ -97,13 +97,32 @@ class CeleryConfig: CELERY_CONFIG = CeleryConfig -FEATURE_FLAGS = {"ALERT_REPORTS": True} +FEATURE_FLAGS = {"ALERT_REPORTS": True,"EMBEDDED_SUPERSET": True,"ENABLE_TEMPLATE_PROCESSING": True,} +# EMBED CODE IFRAME OPTIONS +TALISMAN_ENABLED = False +ENABLE_CORS = True +HTTP_HEADERS={"X-Frame-Options":"ALLOWALL"} + +# Dashboard embedding +GUEST_ROLE_NAME = "Admin" +GUEST_TOKEN_JWT_SECRET = "0f9792d37bddef9d94a1df80e3c7d04f2003b73fdb08edb2781681a73a2e13e67050d07b0c458165fcb8ea0d332ce5e3411aa4b4f5a006dbd5c5728a9a003a45209d386be26fb36f44878156f77ed14379dbc131e353cf2a8f3d876b413bf7d5d90a592b213a6f572cdd74c9b6ddb7e94a948f12be3bc97de0254eda7514e9e9a918f54e8026d4f611ea9a5a12d8a2ffeed51b80dd4030c88fdf6a96ec6860af33294e50bbc816bc8d78fadd649351f7c187637ef2b49ea5aa2c64af99bf6a24ac18518f1fe8bd7c7af00bc27e4026011e2cb2f88eea7ccc32f967708be454219f1b94d323d7664909d16369ec9edcc23afcb0c9de6a6b3168e352c5c1d590f1" Review Comment: ### Hardcoded JWT Secret <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? JWT secret key is hardcoded in the configuration file instead of being sourced from environment variables. ###### Why this matters Hardcoded secrets in source code create security risks and make it difficult to maintain different environments securely. ###### Suggested change ∙ *Feature Preview* Use environment variable for the JWT secret: ```python GUEST_TOKEN_JWT_SECRET = os.getenv('GUEST_TOKEN_JWT_SECRET') if not GUEST_TOKEN_JWT_SECRET: raise ValueError('GUEST_TOKEN_JWT_SECRET environment variable must be set') ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1bbb809-4c1a-4286-935d-97e579566f2d?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:cc5b02e6-475c-4d40-8119-f8c93e99ce33 --> ########## docker/pythonpath_dev/superset_config.py: ########## @@ -97,13 +97,32 @@ class CeleryConfig: CELERY_CONFIG = CeleryConfig -FEATURE_FLAGS = {"ALERT_REPORTS": True} +FEATURE_FLAGS = {"ALERT_REPORTS": True,"EMBEDDED_SUPERSET": True,"ENABLE_TEMPLATE_PROCESSING": True,} +# EMBED CODE IFRAME OPTIONS +TALISMAN_ENABLED = False +ENABLE_CORS = True +HTTP_HEADERS={"X-Frame-Options":"ALLOWALL"} + +# Dashboard embedding +GUEST_ROLE_NAME = "Admin" Review Comment: ### Insecure Guest Access Configuration <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Security configuration that grants Admin role to guest users and disables security protections is dangerous for production environments. ###### Why this matters This configuration could allow unauthorized access to sensitive data and administrative functions, potentially leading to data breaches. ###### Suggested change ∙ *Feature Preview* Implement proper role-based access control for guest users and maintain security headers: ```python TALISMAN_ENABLED = True ENABLE_CORS = False # Enable only if needed with specific origins HTTP_HEADERS={"X-Frame-Options":"SAMEORIGIN"} # Dashboard embedding GUEST_ROLE_NAME = "Viewer" # Use a restricted role ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1912b2d2-b7a5-4300-94ba-1ac9316ce274?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:fa2bb469-0bb4-4fe8-bf30-de4e29c5e851 --> ########## superset-frontend/plugins/plugin-chart-handlebars/src/components/Handlebars/HandlebarsViewer.tsx: ########## @@ -76,28 +96,159 @@ export const HandlebarsViewer = ({ }; // usage: {{dateFormat my_date format="MMMM YYYY"}} -Handlebars.registerHelper('dateFormat', function (context, block) { +hb.registerHelper('dateFormat', function (context: any, block: any) { const f = block.hash.format || 'YYYY-MM-DD'; return moment(context).format(f); }); +hb.registerHelper('parseJSON', (jsonString: string) => { + if (jsonString === undefined) + throw Error('Please call with an object. Example: `parseJSON myObj`'); + return JSON.parse(jsonString); +}); + +hb.registerHelper('inc', (value: string) => { + if (value === undefined) + throw Error('Please call with an object. Example: `inc @index`'); + return parseInt(value, 10) + 1; +}); + // usage: {{ }} -Handlebars.registerHelper('stringify', (obj: any, obj2: any) => { +hb.registerHelper('stringify', (obj: any, obj2: any) => { // calling without an argument if (obj2 === undefined) throw Error('Please call with an object. Example: `stringify myObj`'); return isPlainObject(obj) ? JSON.stringify(obj) : String(obj); }); -Handlebars.registerHelper( - 'formatNumber', - function (number: any, locale = 'en-US') { - if (typeof number !== 'number') { - return number; +hb.registerHelper('formatNumber', function (number: any, locale = 'en-US') { + if (typeof number !== 'number') { + return number; + } + return number.toLocaleString(locale); +}); + +HandlebarsGroupBy.register(hb); + +// Example async function to fetch presigned URLs +async function fetchPresignedUrls0(imageLinks: string[]): Promise<string[]> { + const response = await fetch('/api/v1/presigned_urls/', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ image_links: imageLinks }), + }); + + if (response.ok) { + const data = await response.json(); + return data.presigned_urls; + } + console.error('Error fetching presigned URLs'); + return []; +} + +// Register async helper +hb.registerHelper('getPresignedUrls', async (imageLinks: string[]) => { + const urls = await fetchPresignedUrls0(imageLinks); + const urls1 = JSON.parse(urls as unknown as string); Review Comment: ### Invalid JSON Double Parsing <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Double parsing of JSON response in getPresignedUrls helper. The response from fetchPresignedUrls0 is already parsed with response.json() ###### Why this matters This will cause a runtime error when trying to parse an already parsed JSON object, breaking the presigned URL functionality. ###### Suggested change ∙ *Feature Preview* Remove the unnecessary JSON.parse and directly use the response: ```typescript hb.registerHelper('getPresignedUrls', async (imageLinks: string[]) => { const urls = await fetchPresignedUrls0(imageLinks); const presignedUrls = urls.map( (item: { presigned_url: string }) => item.presigned_url ); return presignedUrls; }); ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a4ff6a5-6ed6-4b74-ad44-4b066484a058?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:61e8bbd4-f5ad-4025-865d-8b5002234c45 --> ########## superset/views/api.py: ########## @@ -126,6 +127,19 @@ def time_range(self, **kwargs: Any) -> FlaskResponse: error_msg = {"message": _("Unexpected time range: %(error)s", error=error)} return self.json_response(error_msg, 400) + @event_logger.log_this + @api + @handle_api_exception + @expose("/v1/presigned_urls/", methods=("POST",)) + def get_presigned_urls(self) -> FlaskResponse: Review Comment: ### Missing API Access Control <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Missing @has_access_api decorator for the presigned URLs endpoint, which is required for API security. ###### Why this matters Without access control, unauthorized users could potentially access and generate presigned URLs, leading to security vulnerabilities. ###### Suggested change ∙ *Feature Preview* Add the @has_access_api decorator: ```python @event_logger.log_this @api @handle_api_exception @has_access_api @expose("/v1/presigned_urls/", methods=("POST",)) def get_presigned_urls(self) -> FlaskResponse: ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d9a8a5d-163f-49c7-88fd-42f437e8e807?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:6475c065-d950-4218-abc8-ec3f19e1887a --> ########## superset-frontend/plugins/plugin-chart-handlebars/src/components/Handlebars/HandlebarsViewer.tsx: ########## @@ -76,28 +96,159 @@ export const HandlebarsViewer = ({ }; // usage: {{dateFormat my_date format="MMMM YYYY"}} -Handlebars.registerHelper('dateFormat', function (context, block) { +hb.registerHelper('dateFormat', function (context: any, block: any) { const f = block.hash.format || 'YYYY-MM-DD'; return moment(context).format(f); }); +hb.registerHelper('parseJSON', (jsonString: string) => { + if (jsonString === undefined) + throw Error('Please call with an object. Example: `parseJSON myObj`'); + return JSON.parse(jsonString); +}); + +hb.registerHelper('inc', (value: string) => { + if (value === undefined) + throw Error('Please call with an object. Example: `inc @index`'); + return parseInt(value, 10) + 1; +}); + // usage: {{ }} -Handlebars.registerHelper('stringify', (obj: any, obj2: any) => { +hb.registerHelper('stringify', (obj: any, obj2: any) => { // calling without an argument if (obj2 === undefined) throw Error('Please call with an object. Example: `stringify myObj`'); return isPlainObject(obj) ? JSON.stringify(obj) : String(obj); }); -Handlebars.registerHelper( - 'formatNumber', - function (number: any, locale = 'en-US') { - if (typeof number !== 'number') { - return number; +hb.registerHelper('formatNumber', function (number: any, locale = 'en-US') { + if (typeof number !== 'number') { + return number; + } + return number.toLocaleString(locale); +}); + +HandlebarsGroupBy.register(hb); + +// Example async function to fetch presigned URLs +async function fetchPresignedUrls0(imageLinks: string[]): Promise<string[]> { + const response = await fetch('/api/v1/presigned_urls/', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ image_links: imageLinks }), + }); + + if (response.ok) { + const data = await response.json(); + return data.presigned_urls; + } + console.error('Error fetching presigned URLs'); + return []; +} + +// Register async helper +hb.registerHelper('getPresignedUrls', async (imageLinks: string[]) => { + const urls = await fetchPresignedUrls0(imageLinks); + const urls1 = JSON.parse(urls as unknown as string); Review Comment: ### Unclear Variable Name and Complex Type Assertion <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Variable name 'urls1' uses a meaningless numeric suffix and the type assertion chain (as unknown as string) is unnecessarily complex. ###### Why this matters Complex type assertions and poor variable naming make the code harder to follow and understand the data flow. ###### Suggested change ∙ *Feature Preview* ```typescript const parsedUrls = JSON.parse(urls.toString()); ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/da8bc4a3-83d9-4f34-b5d8-6a1ca5c3e593?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:a9f7fbae-2b0f-4d73-bcea-8bf157fbb025 --> ########## docker/pythonpath_dev/superset_config.py: ########## @@ -97,13 +97,32 @@ class CeleryConfig: CELERY_CONFIG = CeleryConfig -FEATURE_FLAGS = {"ALERT_REPORTS": True} +FEATURE_FLAGS = {"ALERT_REPORTS": True,"EMBEDDED_SUPERSET": True,"ENABLE_TEMPLATE_PROCESSING": True,} +# EMBED CODE IFRAME OPTIONS +TALISMAN_ENABLED = False +ENABLE_CORS = True +HTTP_HEADERS={"X-Frame-Options":"ALLOWALL"} + +# Dashboard embedding +GUEST_ROLE_NAME = "Admin" +GUEST_TOKEN_JWT_SECRET = "0f9792d37bddef9d94a1df80e3c7d04f2003b73fdb08edb2781681a73a2e13e67050d07b0c458165fcb8ea0d332ce5e3411aa4b4f5a006dbd5c5728a9a003a45209d386be26fb36f44878156f77ed14379dbc131e353cf2a8f3d876b413bf7d5d90a592b213a6f572cdd74c9b6ddb7e94a948f12be3bc97de0254eda7514e9e9a918f54e8026d4f611ea9a5a12d8a2ffeed51b80dd4030c88fdf6a96ec6860af33294e50bbc816bc8d78fadd649351f7c187637ef2b49ea5aa2c64af99bf6a24ac18518f1fe8bd7c7af00bc27e4026011e2cb2f88eea7ccc32f967708be454219f1b94d323d7664909d16369ec9edcc23afcb0c9de6a6b3168e352c5c1d590f1" +GUEST_TOKEN_JWT_ALGO = "HS256" +GUEST_TOKEN_HEADER_NAME = "X-GuestToken" +GUEST_TOKEN_JWT_EXP_SECONDS = 900 # 15 minutes + ALERT_REPORTS_NOTIFICATION_DRY_RUN = True WEBDRIVER_BASEURL = "http://superset:8088/"; # When using docker compose baseurl should be http://superset_app:8088/ # The base URL for the email report hyperlinks. WEBDRIVER_BASEURL_USER_FRIENDLY = WEBDRIVER_BASEURL SQLLAB_CTAS_NO_LIMIT = True + +HTML_SANITIZATION_SCHEMA_EXTENSIONS = { + "attributes": { + "*": ["style","className","script","target"], + }, + "tagNames": ["style","script"], + } Review Comment: ### Unsafe HTML Sanitization Rules <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? HTML sanitization schema allows potentially dangerous script and style tags which could enable XSS attacks. ###### Why this matters Allowing script execution and unrestricted styling in HTML content creates a significant security vulnerability that could be exploited for malicious purposes. ###### Suggested change ∙ *Feature Preview* Remove dangerous tags and attributes from the sanitization schema: ```python HTML_SANITIZATION_SCHEMA_EXTENSIONS = { "attributes": { "*": ["className"], }, "tagNames": [], } ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9b0ddd5-fc4d-4049-92fb-cac2a3b22d4d?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:b865514f-a0e6-4313-8128-afbf34bbac59 --> ########## superset/views/api.py: ########## @@ -126,6 +127,19 @@ def time_range(self, **kwargs: Any) -> FlaskResponse: error_msg = {"message": _("Unexpected time range: %(error)s", error=error)} return self.json_response(error_msg, 400) + @event_logger.log_this + @api + @handle_api_exception + @expose("/v1/presigned_urls/", methods=("POST",)) + def get_presigned_urls(self) -> FlaskResponse: + image_links = request.json.get("image_links") + if not image_links: + return self.json_response({"error": "Image links required"}), 400 + print('*#*#*** in get_presigned_urls') Review Comment: ### Remove Debug Print Statement <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Debug print statement left in production code in the get_presigned_urls endpoint. ###### Why this matters Debug statements in production code can expose sensitive information and pollute logs, making it harder to diagnose real issues. ###### Suggested change ∙ *Feature Preview* Remove the debug print statement: ```python @expose("/v1/presigned_urls/", methods=("POST",)) def get_presigned_urls(self) -> FlaskResponse: image_links = request.json.get("image_links") if not image_links: return self.json_response({"error": "Image links required"}), 400 presigned_urls = get_image_func(image_links) return self.json_response({"presigned_urls": presigned_urls}), 200 ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bdeac3a-34e1-47e4-be8b-f5c001741cb4?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:7fe51e31-23fd-47ce-9827-a57b5cd58be9 --> ########## scripts/build_docker.py: ########## @@ -0,0 +1,294 @@ +#!/usr/bin/env python3 + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import os +import re +import subprocess +from textwrap import dedent + +import click + +REPO = "viveksingh27/bi-superset" +CACHE_REPO = f"{REPO}-cache" +BASE_PY_IMAGE = "3.10-slim-bookworm" + + +def run_cmd(command: str, raise_on_failure: bool = True) -> str: + process = subprocess.Popen( + command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True + ) Review Comment: ### Shell Injection Risk in Command Execution <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Using shell=True with unsanitized input in subprocess.Popen is vulnerable to shell injection attacks. ###### Why this matters If any part of the command string contains user input, an attacker could inject arbitrary shell commands. ###### Suggested change ∙ *Feature Preview* Use command lists instead of shell=True: ```python def run_cmd(command: list[str], raise_on_failure: bool = True) -> str: process = subprocess.Popen( command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True ) ``` </details> <sub> [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b7cd568-b257-4d2f-b1e6-7c2e2b66a672?suggestedFixEnabled=true) 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:015eb96c-22a7-4c1a-ab2a-a6127f3e9d00 --> -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
