feat: encrypt config file. #34197

xiao-77 · 2026-01-07T07:51:04Z

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?

gemini-code-assist · 2026-01-07T07:51:22Z

Summary of Changes

Hello @xiao-77, I'm Gemini Code Assist¹! 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 introduces a significant security enhancement by enabling encryption for configuration files. It achieves this by refactoring the configuration loading and persistence mechanisms to integrate new centralized read/write functions that handle encryption and decryption. This ensures that sensitive configuration data is protected at rest, requiring an encryption key to be loaded before the system can access and apply its settings.

Highlights

Config File Encryption: Implemented encryption and decryption for configuration files by centralizing file I/O operations through new taosReadCfgFile and taosWriteCfgFile functions.
Configuration Loading Refactor: The configuration initialization process (taosInitCfg) has been refactored into distinct taosPreLoadCfg, tryLoadCfgFromDataDir, and taosApplyCfg stages to allow for better control over the loading sequence.
Encryption Key Management: The encryption key loading (dmGetEncryptKey) is now performed earlier in the application startup sequence to ensure keys are available before encrypted configurations are accessed, and taosReadCfgFile explicitly waits for the key.

🧠 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.

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. ↩

gemini-code-assist

Code Review

This pull request introduces encryption for configuration files, a significant security enhancement. The changes primarily involve refactoring configuration file I/O to use new helper functions (taosReadCfgFile and taosWriteCfgFile) that transparently handle encryption and decryption. The application startup sequence has also been adjusted to load encryption keys at the appropriate time. Overall, the changes are well-structured and improve the codebase by centralizing configuration file handling. I have identified a critical syntax error, a high-severity issue related to a redundant function call, and a couple of medium-severity points for improving maintainability and consistency.

gemini-code-assist · 2026-01-07T07:52:36Z

source/dnode/mgmt/exe/dmMain.c

+    taosCloseLog();
+    taosCleanupArgs();
+    return code;
+  };


There is an extraneous semicolon here, which is a syntax error in C. Please remove it.

}

gemini-code-assist · 2026-01-07T07:52:36Z

source/common/src/tglobal.c

  if (!tsc) {
    TAOS_CHECK_GOTO(taosSetTfsCfg(tsCfg), &lino, _exit);
-    TAOS_CHECK_GOTO(tryLoadCfgFromDataDir(tsCfg), &lino, _exit);
  }


The call to taosSetTfsCfg(tsCfg) seems redundant here. It is already called in taosPreLoadCfg under the same !tsc condition. Since taosInitCfg calls taosPreLoadCfg before taosApplyCfg, this second call is unnecessary. Please remove it to avoid redundant work.

gemini-code-assist · 2026-01-07T07:52:36Z

source/common/src/tglobal.c

 #include "cJSON.h"
 #include "defines.h"
 #include "os.h"
 #include "osString.h"
 #include "tconfig.h"
+#include "tencrypt.h"
+#include "tglobal.h"


It's a common C/C++ style convention to include the header file corresponding to the .c/.cpp file first (in this case, tglobal.h in tglobal.c). This helps ensure that the header is self-contained and doesn't have hidden dependencies. Please consider moving #include "tglobal.h" to be the first include after #define _DEFAULT_SOURCE.

#include "tglobal.h" #include "cJSON.h" #include "defines.h" #include "os.h" #include "osString.h" #include "tconfig.h" #include "tencrypt.h"

gemini-code-assist · 2026-01-07T07:52:36Z

source/common/src/tglobal.c

+  if (code != TSDB_CODE_SUCCESS) {
+    cfgCleanup(tsCfg);
+    tsCfg = NULL;
+    (void)printf("failed to pre load cfg at %d since %s\n", lino, tstrerror(code));


printf is used here for error logging. It's better to use the project's logging framework (e.g., uError or dError) for consistency and to ensure logs are correctly routed, especially when running as a daemon where stdout may not be monitored.

uError("failed to pre load cfg at %d since %s", lino, tstrerror(code));

Copilot

Pull request overview

This pull request adds encryption support for configuration files in the TDengine dnode management system. The implementation splits the configuration initialization process into three phases to enable encryption key loading before reading potentially encrypted config files from the data directory.

Key changes:

Refactored config initialization into taosPreLoadCfg, dmGetEncryptKey, tryLoadCfgFromDataDir, and taosApplyCfg sequence
Modified config file I/O functions to use taosReadCfgFile and taosWriteCfgFile for automatic encryption/decryption
Added encryption key wait synchronization in taosReadCfgFile to ensure keys are loaded before reading encrypted files

Reviewed changes

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

File	Description
source/dnode/mgmt/exe/dmMain.c	Restructured startup sequence to load encryption keys before reading config files from data directory
source/common/src/tglobal.c	Refactored config initialization into separate phases and replaced direct file I/O with encryption-aware functions
source/common/src/tencrypt.c	Added encryption key wait logic at the start of config file reading
include/common/tglobal.h	Added declarations for new config initialization phase functions

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

Copilot · 2026-01-07T07:56:32Z

source/common/src/tglobal.c

+  if (!tsc) {
+    TAOS_CHECK_GOTO(taosSetTfsCfg(tsCfg), &lino, _exit);
+  }


The function taosSetTfsCfg is called twice in the new code flow: once in taosPreLoadCfg (line 2597) and again in taosApplyCfg (line 2614). This is redundant since taosApplyCfg is always called after taosPreLoadCfg in the dmMain.c flow. The duplicate call in taosApplyCfg at line 2614 should be sufficient, and the call at line 2597 can be removed to avoid unnecessary work.

Suggested change

if (!tsc) {

TAOS_CHECK_GOTO(taosSetTfsCfg(tsCfg), &lino, _exit);

}

Copilot · 2026-01-07T07:56:32Z

source/dnode/mgmt/exe/dmMain.c

+  if ((code = tryLoadCfgFromDataDir(tsCfg)) != 0) {
+    dError("failed to start since try load config from data dir error");
+    taosCloseLog();
+    taosCleanupArgs();
+    return code;


Missing cleanup: When tryLoadCfgFromDataDir fails at line 542, the error path (lines 543-546) calls taosCloseLog() and taosCleanupArgs() but does not call taosCleanupCfg() to free the tsCfg allocated in taosPreLoadCfg(). This results in a resource leak. The same issue exists in the error path for taosApplyCfg (lines 550-553). Both error paths should include taosCleanupCfg() before returning.

Copilot · 2026-01-07T07:56:32Z

source/common/src/tglobal.c


-  if (taosWriteFile(pFile, serialized, strlen(serialized)) < 0) {
+  // Use taosWriteCfgFile for encryption support (automatically encrypts if tsCfgKey is set)
+  code = taosWriteCfgFile(filename, serialized, strlen(serialized));


Missing null terminator for encrypted files: When encrypting config files with taosWriteCfgFile, the code uses strlen(serialized) which excludes the null terminator. After decryption in taosReadCfgFile, the buffer will not be null-terminated (line 477 returns encryptedDataLen without adding null termination). However, the JSON parser cJSON_Parse called in functions like stypeConfigDeserialize (line 2419 in tglobal.c) requires null-terminated strings. This can cause buffer overruns or parsing failures. The encryption should include the null terminator by using strlen(serialized) + 1 instead of strlen(serialized), or taosReadCfgFile should add explicit null termination for encrypted buffers.

Suggested change

code = taosWriteCfgFile(filename, serialized, strlen(serialized));

// Include the null terminator so decrypted buffers are valid C strings for cJSON_Parse

code = taosWriteCfgFile(filename, serialized, strlen(serialized) + 1);

Copilot · 2026-01-07T07:56:33Z

source/dnode/mgmt/exe/dmMain.c

+  if ((code = taosApplyCfg(configDir, global.envCmd, global.envFile, global.apolloUrl, global.pArgs, 0)) != 0) {
+    dError("failed to start since apply config error");
    taosCloseLog();
    taosCleanupArgs();
    return code;


Missing cleanup: When taosApplyCfg fails at line 549, the error path (lines 550-553) calls taosCloseLog() and taosCleanupArgs() but does not call taosCleanupCfg() to free the tsCfg allocated in taosPreLoadCfg(). This results in a resource leak. The error path should include taosCleanupCfg() before returning.

Copilot · 2026-01-07T07:56:33Z

source/common/src/tencrypt.c

+  if (taosWaitCfgKeyLoaded() != 0) {
+    code = terrno;
+    TSDB_CHECK_CODE(code, lino, _exit);
+  }


Inefficient key wait timing: The function waits for the encryption key to be loaded at line 374 before even checking if the file is encrypted at line 387. This causes unnecessary waiting and potential timeouts when reading unencrypted config files. The wait should be moved to after the encryption check (around line 406) and only performed when isEncrypted is true, improving performance for unencrypted files and avoiding spurious timeout errors.

Copilot · 2026-01-07T07:56:33Z

source/common/src/tglobal.c

+  code = taosWriteCfgFile(filename, serializedCfg, strlen(serializedCfg));
+  if (code != TSDB_CODE_SUCCESS) {


Missing null terminator for encrypted files: When encrypting config files with taosWriteCfgFile, the code uses strlen(serializedCfg) which excludes the null terminator. After decryption in taosReadCfgFile, the buffer will not be null-terminated. However, the JSON parser cJSON_Parse called in cfgDeserialize requires null-terminated strings. This can cause buffer overruns or parsing failures. The encryption should include the null terminator by using strlen(serializedCfg) + 1 instead of strlen(serializedCfg).

Copilot · 2026-01-07T07:56:34Z

source/common/src/tglobal.c

+  code = taosWriteCfgFile(stypeFilename, serializedStype, strlen(serializedStype));
+  if (code != TSDB_CODE_SUCCESS) {


Missing null terminator for encrypted files: When encrypting config files with taosWriteCfgFile, the code uses strlen(serializedStype) which excludes the null terminator. After decryption in taosReadCfgFile, the buffer will not be null-terminated. However, the JSON parser cJSON_Parse called in stypeConfigDeserialize requires null-terminated strings. This can cause buffer overruns or parsing failures. The encryption should include the null terminator by using strlen(serializedStype) + 1 instead of strlen(serializedStype).

Copilot · 2026-01-07T07:56:34Z

source/dnode/mgmt/exe/dmMain.c

+    taosCloseLog();
+    taosCleanupArgs();
+    return code;
+  };


Unnecessary semicolon after closing brace at line 540. While syntactically valid, this is inconsistent with the surrounding code style (see lines 533, 547, 554) and should be removed for consistency.

Suggested change

};

}

feat: encrypt config file.

7109b00

Copilot AI review requested due to automatic review settings January 7, 2026 07:51

xiao-77 requested review from dapan1121, guanshengliang and hzcheng as code owners January 7, 2026 07:51

taosdata-bot bot added the team engine label Jan 7, 2026

Copilot started reviewing on behalf of xiao-77 January 7, 2026 07:51 View session

gemini-code-assist bot reviewed Jan 7, 2026

View reviewed changes

Copilot AI reviewed Jan 7, 2026

View reviewed changes

feat: do not encrypt dnode.info.

88e4ef9

	if (!tsc) {
	TAOS_CHECK_GOTO(taosSetTfsCfg(tsCfg), &lino, _exit);
	}

	code = taosWriteCfgFile(filename, serialized, strlen(serialized));
	// Include the null terminator so decrypted buffers are valid C strings for cJSON_Parse
	code = taosWriteCfgFile(filename, serialized, strlen(serialized) + 1);

		code = taosWriteCfgFile(filename, serializedCfg, strlen(serializedCfg));
		if (code != TSDB_CODE_SUCCESS) {

		code = taosWriteCfgFile(stypeFilename, serializedStype, strlen(serializedStype));
		if (code != TSDB_CODE_SUCCESS) {

feat: encrypt config file. #34197

Are you sure you want to change the base?

feat: encrypt config file. #34197

Conversation

xiao-77 commented Jan 7, 2026

Description

Issue(s)

Checklist

Uh oh!

gemini-code-assist bot commented Jan 7, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants