Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/components/settings/settings-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from 'react';
import type { SaveButtonRenderProps, SettingsElement } from './settings-types';
import {
buildIdIndex,
evaluateDependencies,
extractValues,
formatSettingsData,
Expand Down Expand Up @@ -355,12 +356,18 @@ export function SettingsProvider({
[schema, onChange, keyToScopeMap, activeSubpage, activePage]
);

// Field-id → dependency_key index, used as a fallback when a dependency
// declares its `key` as a plain field id instead of the reconstructed
// dot-path. Memoized on the schema since ids and dep_keys don't change
// at runtime.
const idIndex = useMemo(() => buildIdIndex(schema), [schema]);

// Dependency evaluation
const shouldDisplay = useCallback(
(element: SettingsElement): boolean => {
return evaluateDependencies(element, values);
return evaluateDependencies(element, values, idIndex);
},
[values]
[values, idIndex]
);

// Navigation helpers
Expand Down
67 changes: 65 additions & 2 deletions src/components/settings/settings-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,64 @@ export function extractValues(
return values;
}

/**
* Builds a `{ field_id: dependency_key }` map from a hierarchical schema.
*
* Used by `evaluateDependencies` to resolve a dependency `key` that is
* declared as a plain field id (e.g. `"product_info_generate"`) instead
* of the full reconstructed dot-path (e.g.
* `"product_generation.product_image_section.product_info_generate"`).
*
* Consumers may pass id-keyed dependencies when their backend already
* guarantees globally-unique field ids; the dot-path remains supported
* for backwards compatibility.
*/
export function buildIdIndex(
schema: SettingsElement[]
): Record<string, string> {
const idIndex: Record<string, string> = {};

const walk = (elements: SettingsElement[]) => {
for (const el of elements) {
if (
el.type === 'field' &&
el.id &&
el.dependency_key &&
el.id !== el.dependency_key
) {
// First writer wins — if two fields share an id (a schema
// bug consumers should detect with their own validator),
// we don't silently clobber the earlier mapping.
if (!(el.id in idIndex)) {
idIndex[el.id] = el.dependency_key;
}
Comment on lines +312 to +327
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify prototype-sensitive dictionary usage in the touched file.
rg -n -C2 "Record<string, string> = \{\}|\\bin idIndex\\b|idIndex\\[dep\\.key\\]" src/components/settings/settings-formatter.ts

Repository: getdokan/plugin-ui

Length of output: 1073


Use own-property checks for idIndex to avoid prototype-key collisions.

Using a plain object {} for the idIndex dictionary with the in operator (line 325) and direct property access (line 369) is unsafe. If an ID matches a prototype property like constructor, toString, or __proto__, the in check will incorrectly return true, skipping the mapping, and direct property reads will return inherited values instead of undefined.

Use Object.create(null) and Object.prototype.hasOwnProperty.call() for safe own-property checks.

Proposed fix
-export function buildIdIndex(
-    schema: SettingsElement[]
-): Record<string, string> {
-    const idIndex: Record<string, string> = {};
+export function buildIdIndex(
+    schema: SettingsElement[]
+): Record<string, string> {
+    const idIndex = Object.create(null) as Record<string, string>;
@@
-                if (!(el.id in idIndex)) {
+                if (!Object.prototype.hasOwnProperty.call(idIndex, el.id)) {
                     idIndex[el.id] = el.dependency_key;
                 }
@@
-        if (currentValue === undefined && idIndex) {
-            const resolved = idIndex[dep.key];
-            if (resolved !== undefined) {
+        if (
+            currentValue === undefined &&
+            idIndex &&
+            Object.prototype.hasOwnProperty.call(idIndex, dep.key)
+        ) {
+            const resolved = idIndex[dep.key];
+            if (resolved !== undefined) {
                 currentValue = values[resolved];
             }
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/settings/settings-formatter.ts` around lines 312 - 327, The
idIndex map should not be a plain {} because prototype keys can collide; change
its creation to a prototypeless map (use Object.create(null)) and replace any
"el.id in idIndex" checks and any direct property reads with own-property checks
(e.g., Object.prototype.hasOwnProperty.call(idIndex, el.id) or
Object.hasOwn(idIndex, el.id)) before treating or reading idIndex[el.id]; update
the checks inside the walk function and wherever idIndex is read (referencing
idIndex and the walk / SettingsElement logic) to use these own-property checks
so prototype properties like "toString" or "constructor" won't be treated as
existing IDs.

}
if (el.children && el.children.length > 0) {
walk(el.children);
}
}
};

walk(schema);
return idIndex;
}

/**
* Evaluates whether a field should be displayed based on its dependencies.
*
* Dependency `key` resolution:
* 1. Look up `values[dep.key]` directly (existing dot-path behavior).
* 2. If that yields `undefined` and an `idIndex` is supplied, treat
* `dep.key` as a plain field id and resolve via `idIndex[dep.key]`
* to its real `dependency_key` before reading from `values`.
*
* The `idIndex` is optional. When omitted, behavior is identical to the
* previous version of this function.
*/
export function evaluateDependencies(
element: SettingsElement,
values: Record<string, any>
values: Record<string, any>,
idIndex?: Record<string, string>
): boolean {
if (!element.dependencies || element.dependencies.length === 0) {
return element.display !== false;
Expand All @@ -308,7 +360,18 @@ export function evaluateDependencies(
return element.dependencies.every((dep) => {
if (!dep.key) return true;

const currentValue = values[dep.key];
let currentValue = values[dep.key];

// Field-id fallback: dep.key may be a plain id (not a dot-path).
// Resolve it through the idIndex to the underlying dependency_key
// and re-read from values.
if (currentValue === undefined && idIndex) {
const resolved = idIndex[dep.key];
if (resolved !== undefined) {
currentValue = values[resolved];
}
}

const comparison = dep.comparison || '==';
const expectedValue = dep.value;
const effect = dep.effect || 'show';
Expand Down
Loading