bito-code-review[bot] commented on code in PR #44955:
URL: https://github.com/apache/superset/pull/44955#discussion_r4192734592
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts:
##########
@@ -329,22 +358,52 @@ export function syncTimeComparisonGroups(
* Remap saved comparison-column config onto locale-independent keys.
*
* Charts created before comparison slots were stored as `Main …` may still
- * keep `t('Main') …` entries (for example `Principale revenue`). The column
- * editor lists the stored keys, so those legacy entries must be rewritten
- * before it filters `column_config` down to `colnames`.
+ * keep `t('Main') …` entries (for example `Principale revenue`). A buggy
+ * column editor also wrote `COLUMN_NAME_ALIASES` display names (for example
+ * `Time` for `__timestamp`). The column editor lists the stored keys, so
+ * those legacy entries must be rewritten before it filters `column_config`
+ * down to `colnames`.
*/
export function normalizeColumnConfigKeys<T>(
value: Record<string, T> | null | undefined,
colnames: string[],
+ knownKeys: string[] = colnames,
): Record<string, T> {
if (!value) {
return {};
}
const colnamesSet = new Set(colnames);
+ const knownKeySet = new Set(knownKeys);
+ const aliasToKey = new Map<string, string>();
+ const ambiguousAliases = new Set<string>();
+ colnames.forEach(col => {
+ getColumnNameAliasLabels(col).forEach(alias => {
+ // A saved key that names a real column or metric is not a legacy alias.
+ if (!alias || knownKeySet.has(alias) || ambiguousAliases.has(alias)) {
+ return;
+ }
+ const existing = aliasToKey.get(alias);
+ if (existing && existing !== col) {
+ aliasToKey.delete(alias);
+ ambiguousAliases.add(alias);
+ return;
+ }
+ aliasToKey.set(alias, col);
+ });
+ });
const next: Record<string, T> = {};
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-1321: Prevent prototype pollution via object
key</b></div>
<div id="fix">
The assignment `next[key] = config` uses a key derived from user-controlled
input, which can lead to prototype pollution
([CWE-1321](https://cwe.mitre.org/data/definitions/1321.html)) if the key is
`__proto__`, `constructor`, or `prototype`. Initialize `next` with a
null-prototype object to mitigate this risk.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
const next: Record<string, T> = Object.create(null) as Record<string, T>;
````
</div>
</details>
</div>
<small><i>Code Review Run #17c4fa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
