codeant-ai-for-open-source[bot] commented on code in PR #43396:
URL: https://github.com/apache/superset/pull/43396#discussion_r3831864679
##########
superset/utils/excel.py:
##########
@@ -47,20 +47,28 @@
FORMULA_PREFIXES = {"=", "+", "-", "@"}
+def _quote_formula(value: Any) -> Any:
+ """Prefix a string with a quote when it would parse as a formula."""
+ return (
+ f"'{value}"
+ if isinstance(value, str) and len(value) and value[0] in
FORMULA_PREFIXES
+ else value
+ )
+
+
def quote_formulas(df: pd.DataFrame) -> pd.DataFrame:
"""
Make sure to quote any formulas for security reasons.
"""
for col in df.select_dtypes(include="object").columns:
- df[col] = df[col].apply(
- lambda x: (
- f"'{x}"
- if isinstance(x, str) and len(x) and x[0] in FORMULA_PREFIXES
- else x
- )
- )
-
- return df
+ df[col] = df[col].apply(_quote_formula)
+
+ # Column headers and index labels are written to the sheet as well, and
+ # pivot exports promote data values into both (a hostile warehouse string
+ # can become a header or row label), so quote them like the CSV writer
+ # quotes its headers. ``rename`` applies the mapper to every level of a
+ # MultiIndex.
+ return df.rename(columns=_quote_formula, index=_quote_formula)
Review Comment:
**Suggestion:** The rename mapper sanitizes index labels and column labels,
but it does not sanitize `df.index.name` or the names of MultiIndex levels.
Pandas writes these names as header cells in the worksheet, so a
warehouse-controlled name such as `=HYPERLINK(...)` can still be interpreted as
a formula. Sanitize index and level names before calling `to_excel`. [security]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ Formula injection remains possible through exported index headers.
- โ ๏ธ Pivoted XLSX exports expose unsanitized level-name cells.
- โ ๏ธ Victims opening affected workbooks may execute spreadsheet formulas.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/excel.py
**Line:** 71:71
**Comment:**
*Security: The rename mapper sanitizes index labels and column labels,
but it does not sanitize `df.index.name` or the names of MultiIndex levels.
Pandas writes these names as header cells in the worksheet, so a
warehouse-controlled name such as `=HYPERLINK(...)` can still be interpreted as
a formula. Sanitize index and level names before calling `to_excel`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=dd9e188a5cf1e0dbbf14eef16e084bcad20efc388f11174e66da16df8d3a56f5&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=dd9e188a5cf1e0dbbf14eef16e084bcad20efc388f11174e66da16df8d3a56f5&reaction=dislike'>๐</a>
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -46,6 +52,27 @@ def resample(
_("Resample method should be in ") + ", ".join(RESAMPLE_METHOD) +
"."
)
+ if len(df):
+ try:
+ step = pd.Timedelta(pd.tseries.frequencies.to_offset(rule))
+ except ValueError:
+ # Non-fixed frequencies (month, quarter, year) have no fixed
+ # Timedelta; their projected row count is bounded by the span in
+ # days and needs no cap. Invalid rules fail in ``df.resample``.
+ step = None
+ if step is not None and step.value > 0:
+ span = df.index.max() - df.index.min()
+ projected_rows = span.value // step.value + 1
+ if projected_rows > MAX_RESAMPLE_ROWS:
Review Comment:
**Suggestion:** The estimate uses only the observed span and does not
account for resampling-bin alignment. Pandas can create a bin before the first
timestamp or after the last timestamp, so the actual result can contain more
rows than `projected_rows` and exceed `MAX_RESAMPLE_ROWS` despite passing this
check. Calculate the projected range using the same bin boundaries pandas will
use, or include the alignment margin in the limit check. [logic error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Resample output can exceed the configured one-million-row limit.
- โ ๏ธ Large intermediate DataFrames remain possible on resample requests.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/pandas_postprocessing/resample.py
**Line:** 64:66
**Comment:**
*Logic Error: The estimate uses only the observed span and does not
account for resampling-bin alignment. Pandas can create a bin before the first
timestamp or after the last timestamp, so the actual result can contain more
rows than `projected_rows` and exceed `MAX_RESAMPLE_ROWS` despite passing this
check. Calculate the projected range using the same bin boundaries pandas will
use, or include the alignment margin in the limit check.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=b266d4b7fc95cfa8b7883f116aa4faabc37e4cb9b102c09e219f0d71269caccc&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=b266d4b7fc95cfa8b7883f116aa4faabc37e4cb9b102c09e219f0d71269caccc&reaction=dislike'>๐</a>
##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -45,6 +54,14 @@ def histogram(
and each column corresponds to a histogram bin. The values are
the counts in each bin.
""" # noqa: E501
+ if not isinstance(bins, int) or not 1 <= bins <= MAX_HISTOGRAM_BINS:
Review Comment:
**Suggestion:** `bool` is a subclass of `int` in Python, so `bins=True`
passes this validation and is treated as one bin by NumPy even though the
option contract requires an integer bin count. Reject booleans explicitly
before applying the numeric bounds. [type error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Invalid histogram configuration silently becomes one bin.
- โ ๏ธ Direct chart-data requests receive inconsistent option validation.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/pandas_postprocessing/histogram.py
**Line:** 57:57
**Comment:**
*Type Error: `bool` is a subclass of `int` in Python, so `bins=True`
passes this validation and is treated as one bin by NumPy even though the
option contract requires an integer bin count. Reject booleans explicitly
before applying the numeric bounds.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=0d6dcd4ec4a489a98e2227d4f83c5dfd918fc5344810c076d1b6b93fcea848cf&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43396&comment_hash=0d6dcd4ec4a489a98e2227d4f83c5dfd918fc5344810c076d1b6b93fcea848cf&reaction=dislike'>๐</a>
--
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]
