-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: use 0600 permissions on config.yaml #8256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 4 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="extensions/cli/src/onboarding.ts">
<violation number="1" location="extensions/cli/src/onboarding.ts:46">
The `createOrUpdateConfig` function duplicates `extensions/cli/src/freeTrialTransition.ts:createOrUpdateConfig()` function. This function handles sensitive configuration writing, and its duplication increases maintenance burden and potential for inconsistencies in how API keys are managed.</violation>
<violation number="2" location="extensions/cli/src/onboarding.ts:46">
Setting the mode option only affects freshly created files; when config.yaml already exists, this write leaves its previous permissions (e.g., 0644) intact, so the update path still exposes the API key to other users. Please chmod the file after writing to enforce 0600 on updates.</violation>
</file>
<file name="core/util/paths.ts">
<violation number="1" location="core/util/paths.ts:264">
Passing { mode: 0o600 } to writeFileSync does not update permissions of an existing file, so editing config.yaml leaves prior (possibly insecure) permissions unchanged. Please explicitly chmod the file after writing to ensure it is 0600.</violation>
</file>
<file name="extensions/cli/src/freeTrialTransition.ts">
<violation number="1" location="extensions/cli/src/freeTrialTransition.ts:34">
Setting mode in writeFileSync does not tighten permissions when config.yaml already exists, so existing configs remain readable by group/others despite this change; ensure the file is chmod 0o600 after writing.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
extensions/cli/src/onboarding.ts
Outdated
|
|
||
| const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createOrUpdateConfig function duplicates extensions/cli/src/freeTrialTransition.ts:createOrUpdateConfig() function. This function handles sensitive configuration writing, and its duplication increases maintenance burden and potential for inconsistencies in how API keys are managed.
Prompt for AI agents
Address the following comment on extensions/cli/src/onboarding.ts at line 46:
<comment>The `createOrUpdateConfig` function duplicates `extensions/cli/src/freeTrialTransition.ts:createOrUpdateConfig()` function. This function handles sensitive configuration writing, and its duplication increases maintenance burden and potential for inconsistencies in how API keys are managed.</comment>
<file context>
@@ -43,7 +43,7 @@ export async function createOrUpdateConfig(apiKey: string): Promise<void> {
const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey);
- fs.writeFileSync(CONFIG_PATH, updatedContent);
+ fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 });
}
</file context>
core/util/paths.ts
Outdated
| if (typeof configYaml === "object" && configYaml !== null) { | ||
| configYaml = callback(configYaml as any) as any; | ||
| fs.writeFileSync(getConfigYamlPath(), YAML.stringify(configYaml)); | ||
| fs.writeFileSync(configPath, YAML.stringify(configYaml), { mode: 0o600 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing { mode: 0o600 } to writeFileSync does not update permissions of an existing file, so editing config.yaml leaves prior (possibly insecure) permissions unchanged. Please explicitly chmod the file after writing to ensure it is 0600.
Prompt for AI agents
Address the following comment on core/util/paths.ts at line 264:
<comment>Passing { mode: 0o600 } to writeFileSync does not update permissions of an existing file, so editing config.yaml leaves prior (possibly insecure) permissions unchanged. Please explicitly chmod the file after writing to ensure it is 0600.</comment>
<file context>
@@ -255,12 +255,13 @@ function editConfigJson(
if (typeof configYaml === "object" && configYaml !== null) {
configYaml = callback(configYaml as any) as any;
- fs.writeFileSync(getConfigYamlPath(), YAML.stringify(configYaml));
+ fs.writeFileSync(configPath, YAML.stringify(configYaml), { mode: 0o600 });
} else {
console.warn("config.yaml is not a valid object");
</file context>
✅ Addressed in 2a6a0e9
extensions/cli/src/onboarding.ts
Outdated
|
|
||
| const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the mode option only affects freshly created files; when config.yaml already exists, this write leaves its previous permissions (e.g., 0644) intact, so the update path still exposes the API key to other users. Please chmod the file after writing to enforce 0600 on updates.
Prompt for AI agents
Address the following comment on extensions/cli/src/onboarding.ts at line 46:
<comment>Setting the mode option only affects freshly created files; when config.yaml already exists, this write leaves its previous permissions (e.g., 0644) intact, so the update path still exposes the API key to other users. Please chmod the file after writing to enforce 0600 on updates.</comment>
<file context>
@@ -43,7 +43,7 @@ export async function createOrUpdateConfig(apiKey: string): Promise<void> {
const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey);
- fs.writeFileSync(CONFIG_PATH, updatedContent);
+ fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 });
}
</file context>
✅ Addressed in 2a6a0e9
|
|
||
| const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent); | ||
| fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting mode in writeFileSync does not tighten permissions when config.yaml already exists, so existing configs remain readable by group/others despite this change; ensure the file is chmod 0o600 after writing.
Prompt for AI agents
Address the following comment on extensions/cli/src/freeTrialTransition.ts at line 34:
<comment>Setting mode in writeFileSync does not tighten permissions when config.yaml already exists, so existing configs remain readable by group/others despite this change; ensure the file is chmod 0o600 after writing.</comment>
<file context>
@@ -31,7 +31,7 @@ async function createOrUpdateConfig(apiKey: string): Promise<void> {
const updatedContent = updateAnthropicModelInYaml(existingContent, apiKey);
- fs.writeFileSync(CONFIG_PATH, updatedContent);
+ fs.writeFileSync(CONFIG_PATH, updatedContent, { mode: 0o600 });
}
</file context>
✅ Addressed in 2a6a0e9
|
🎉 This PR is included in version 1.29.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.26.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Add 0600 file permission (only owner can read and write) to config.yaml when creating a new one or updating it (in both cli and extension).
closes CON-4236
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
[ When applicable, please include a short screen recording or screenshot - this makes it much easier for us as contributors to review and understand your changes. See this PR as a good example. ]
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Set config.yaml to 0600 on creation and updates across core, CLI, and VS Code so only the owner can read/write. Meets Linear issue CON-4236.