Skip to content

enh: privilege parse and check create stream#34143

Merged
guanshengliang merged 6 commits intomainfrom
enh/TS-7232-main
Jan 4, 2026
Merged

enh: privilege parse and check create stream#34143
guanshengliang merged 6 commits intomainfrom
enh/TS-7232-main

Conversation

@kailixu
Copy link
Copy Markdown
Contributor

@kailixu kailixu commented Dec 31, 2025

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

Copilot AI review requested due to automatic review settings December 31, 2025 08:22
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @kailixu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on strengthening the security and correctness of privilege management within the system. It introduces more robust authorization checks for CREATE STREAM statements, ensuring that users have the necessary permissions on both the source and target database objects. Additionally, it refines the internal representation and parsing of ALTER ROLE statements to accurately handle different types of role modifications, improving data integrity and system reliability.

Highlights

  • Enhanced Privilege Checks for CREATE STREAM: Added more granular privilege checks for CREATE STREAM statements, ensuring that users possess SELECT privilege on the source table and USE privilege on its associated database.
  • Refined ALTER ROLE Statement Handling: Modified the JSON serialization and deserialization logic for GRANT statements to correctly differentiate and process ALTER ROLE operations, handling both privilege sets and role names based on the optrType field.
  • Conditional Target Database Privilege Checks: Adjusted privilege checks for the target database during CREATE STREAM operations, making them conditional on whether a target database name is explicitly provided.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances privilege parsing and checking for CREATE STREAM operations by adding conditional logic for optional database targets and improving grant statement serialization.

Key Changes:

  • Added conditional privilege checks for optional targetDbName in stream creation, preventing unnecessary authorization when target database is not specified
  • Added privilege check for trigger table database access (PRIV_DB_USE)
  • Improved grant statement serialization to support multiple operation types (privileges vs roles) with proper conditional handling

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
source/libs/parser/src/parAuthenticator.c Added PRIV_DB_USE check for trigger table database and conditional targetDbName privilege checks
source/libs/parser/src/parAstParser.c Reorganized privilege reservation calls and added conditional logic for targetDbName
source/libs/nodes/src/nodesCodeFuncs.c Enhanced grant statement serialization with tabName and optrType support, improved privilege parsing with strsplit
Comments suppressed due to low confidence (1)

source/libs/nodes/src/nodesCodeFuncs.c:9362

  • The changes to grant statement serialization logic (adding tabName and optrType fields, changing privilege parsing) lack test coverage. Consider adding tests to verify that serialization and deserialization work correctly for all cases, including when tabName is empty and for different optrType values (TSDB_ALTER_ROLE_PRIVILEGES and TSDB_ALTER_ROLE_ROLE).
static int32_t grantStmtToJson(const void* pObj, SJson* pJson) {
  const SGrantStmt* pNode = (const SGrantStmt*)pObj;

  int32_t code = tjsonAddStringToObject(pJson, jkGrantStmtUserName, pNode->principal);
  if (TSDB_CODE_SUCCESS == code) {
    code = tjsonAddStringToObject(pJson, jkGrantStmtObjName, pNode->objName);
  }
  if (TSDB_CODE_SUCCESS == code) {
    if (pNode->tabName[0] != '\0') {
      code = tjsonAddStringToObject(pJson, jkNameTableName, pNode->tabName);
    }
  }
  if (TSDB_CODE_SUCCESS == code) {
    code = tjsonAddIntegerToObject(pJson, "optrType", pNode->optrType);
  }

  if (TSDB_CODE_SUCCESS == code) {
    if (pNode->optrType == TSDB_ALTER_ROLE_PRIVILEGES) {
      char    privSet[PRIV_GROUP_CNT * 20] = {0};
      int32_t len = 0;
      for (int32_t i = 0; i < PRIV_GROUP_CNT; ++i) {
        len += snprintf(privSet + len, sizeof(privSet) - len, "%" PRIu64 ",", pNode->privileges.privSet.set[i]);
      }
      if (len > 0) privSet[len - 1] = '\0';  // remove last comma
      code = tjsonAddStringToObject(pJson, jkGrantStmtPrivileges, privSet);
    } else if (pNode->optrType == TSDB_ALTER_ROLE_ROLE) {
      code = tjsonAddStringToObject(pJson, jkRoleStmtRoleName, pNode->roleName);
    }
  }

  return code;
}

static int32_t jsonToGrantStmt(const SJson* pJson, void* pObj) {
  SGrantStmt* pNode = (SGrantStmt*)pObj;

  int32_t code = tjsonGetStringValue(pJson, jkGrantStmtUserName, pNode->principal);
  if (TSDB_CODE_SUCCESS == code) {
    code = tjsonGetStringValue(pJson, jkGrantStmtObjName, pNode->objName);
  }
  if (TSDB_CODE_SUCCESS == code) {
    code = tjsonGetStringValue(pJson, jkNameTableName, pNode->tabName);
  }
  if (TSDB_CODE_SUCCESS == code) {
    tjsonGetNumberValue(pJson, "optrType", pNode->optrType, code);
  }
  if (TSDB_CODE_SUCCESS == code) {
    if (pNode->optrType == TSDB_ALTER_ROLE_PRIVILEGES) {
      char privSet[256] = {0};
      code = tjsonGetStringValue2(pJson, jkGrantStmtPrivileges, privSet, sizeof(privSet));
      if (TSDB_CODE_SUCCESS == code) {
        int32_t split_num = 0;
        char**  split = strsplit(privSet, ",", &split_num);
        if (!split) {
          nodesError("failed at line %d to parse privileges string: %s", __LINE__, privSet);
          return TSDB_CODE_INVALID_PARA;
        }
        for (int32_t i = 0; i < PRIV_GROUP_CNT; ++i) {
          if (i < split_num) {
            uint64_t value = 0;
            if ((code = taosStr2Uint64(split[i], &value))) {
              taosMemoryFree(split);
              return code;
            }
            pNode->privileges.privSet.set[i] = value;
          } else {
            pNode->privileges.privSet.set[i] = 0;
          }
        }
        taosMemoryFree(split);
      }
    } else if (pNode->optrType == TSDB_ALTER_ROLE_ROLE) {
      code = tjsonGetStringValue(pJson, jkRoleStmtRoleName, pNode->roleName);
    }
  }

  return code;
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/libs/nodes/src/nodesCodeFuncs.c Outdated
Comment thread source/libs/nodes/src/nodesCodeFuncs.c
Comment thread source/libs/nodes/src/nodesCodeFuncs.c
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances privilege parsing and checking for CREATE STREAM statements. The changes in parAstParser.c and parAuthenticator.c correctly add privilege checks for CREATE STREAM operations, including handling cases where a target table is not specified. However, a critical security vulnerability has been introduced in nodesCodeFuncs.c by removing a buffer overflow check when serializing privileges to JSON. Additionally, there is some redundant code in parAstParser.c that should be cleaned up.

Comment thread source/libs/nodes/src/nodesCodeFuncs.c
Comment thread source/libs/parser/src/parAstParser.c
@kailixu kailixu requested a review from a team as a code owner January 4, 2026 02:28
@guanshengliang guanshengliang merged commit 5b07ea1 into main Jan 4, 2026
19 of 22 checks passed
@guanshengliang guanshengliang deleted the enh/TS-7232-main branch January 4, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants