codeant-ai-for-open-source[bot] commented on code in PR #40466:
URL: https://github.com/apache/superset/pull/40466#discussion_r3324578390


##########
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx:
##########
@@ -42,22 +46,40 @@ export const OAuth2ClientField = ({
   changeMethods,
   db,
   default_value: defaultValue,
+  isPublic = true,
 }: FieldPropTypes) => {
-  const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}');
-  const [oauth2ClientInfo, setOauth2ClientInfo] = useState<OAuth2ClientInfo>({
-    id: encryptedExtra.oauth2_client_info?.id || '',
-    secret: encryptedExtra.oauth2_client_info?.secret || '',
-    authorization_request_uri:
-      encryptedExtra.oauth2_client_info?.authorization_request_uri ||
-      defaultValue?.authorization_request_uri ||
-      '',
-    token_request_uri:
-      encryptedExtra.oauth2_client_info?.token_request_uri ||
-      defaultValue?.token_request_uri ||
-      '',
-    scope:
-      encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '',
-  });
+  const deriveOauth2ClientInfo = (): OAuth2ClientInfo => {
+    const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}');
+    return {
+      id: encryptedExtra.oauth2_client_info?.id || '',
+      secret: encryptedExtra.oauth2_client_info?.secret || '',
+      authorization_request_uri:
+        encryptedExtra.oauth2_client_info?.authorization_request_uri ||
+        defaultValue?.authorization_request_uri ||
+        '',
+      token_request_uri:
+        encryptedExtra.oauth2_client_info?.token_request_uri ||
+        defaultValue?.token_request_uri ||
+        '',
+      scope:
+        encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '',
+    };
+  };

Review Comment:
   **Suggestion:** `masked_encrypted_extra` is parsed and then dereferenced 
without validating the parsed shape. If the backend returns malformed JSON 
(throws) or the valid JSON literal `null`, this component crashes during render 
(`Cannot read properties of null`). Parse defensively and default to an empty 
object when parsing fails or when the parsed value is not a plain object. [null 
pointer]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Editing OAuth2-enabled database can crash connection modal.
   - ⚠️ Prevents updating Google Sheets OAuth2 client settings.
   - ⚠️ Unhandled JSON parse errors degrade admin UX.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open the database connection modal, which renders `DatabaseModal` from
   `superset-frontend/src/features/databases/DatabaseModal/index.tsx:70-82`; 
this uses
   `DatabaseConnectionForm` (import at line 12) to render the dynamic form for 
a database.
   
   2. In `DatabaseConnectionForm`
   
(`superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx:87-137`),
   note that fields are rendered from `FORM_FIELD_MAP`, and
   `FORM_FIELD_MAP.oauth2_client_info = OAuth2ClientField` in 
`constants.ts:72-80`, so any
   database engine whose parameters schema exposes `oauth2_client_info` will 
render
   `OAuth2ClientField`.
   
   3. Construct or load a `DatabaseObject` (type defined in
   `superset-frontend/src/features/databases/types.ts:67-124`) whose 
`masked_encrypted_extra`
   property is the JSON literal string `"null"` (for example, by adding a new 
Jest test in
   `OAuth2ClientField.test.tsx` that sets 
`defaultProps.db.masked_encrypted_extra = 'null'`
   before calling `render(<OAuth2ClientField {...defaultProps} />)` as in lines 
74-79).
   
   4. When React renders `OAuth2ClientField` (`OAuth2ClientField.tsx:45-50`), 
the state
   initializer `useState(deriveOauth2ClientInfo)` invokes 
`deriveOauth2ClientInfo` (line 51),
   which executes `JSON.parse(db?.masked_encrypted_extra || '{}')` (line 52); 
for `"null"`
   this returns `null`, and the following line 
`encryptedExtra.oauth2_client_info?.id` (line
   54) attempts to access a property on `null`, causing a runtime error 
(`Cannot read
   properties of null (reading 'oauth2_client_info')`) and crashing the form 
render.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb0d8b702f7849e4bc1cf00a4cc5c604&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eb0d8b702f7849e4bc1cf00a4cc5c604&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
   **Line:** 51:67
   **Comment:**
        *Null Pointer: `masked_encrypted_extra` is parsed and then dereferenced 
without validating the parsed shape. If the backend returns malformed JSON 
(throws) or the valid JSON literal `null`, this component crashes during render 
(`Cannot read properties of null`). Parse defensively and default to an empty 
object when parsing fails or when the parsed value is not a plain object.
   
   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%2F40466&comment_hash=f8fb332d848c2383eb767df4ec0a9611fe087a2971472627446908d1768d28f7&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40466&comment_hash=f8fb332d848c2383eb767df4ec0a9611fe087a2971472627446908d1768d28f7&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]

Reply via email to