Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions .changeset/quiet-bash-permissions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---

Respect deleted bash permission overrides after upgrading.
21 changes: 16 additions & 5 deletions packages/opencode/src/kilocode/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,23 +404,32 @@ export namespace KilocodeConfig {

/** Global config file names in read-merge order (lowest-to-highest precedence). */
export const GLOBAL_CONFIG_FILES = ["config.json", "kilo.json", "kilo.jsonc", "opencode.json", "opencode.jsonc"]
const BASH_PERMISSION_MIGRATION = ".bash-permission-migrated"

/**
* Migrate bash permission for existing users before config is consumed.
*
* Existing users (those with at least one global config file or the legacy TOML
* config) who have no explicit `permission.bash` setting get `bash: "allow"`
* written to their highest-precedence config file. This preserves their current
* behavior now that the new default is `bash: "ask"`.
* behavior now that the new default is `bash: "ask"`. A completion marker makes
* the migration idempotent so subsequent user edits are not migrated again.
*/
export async function migrateBashPermission() {
const marker = path.join(Global.Path.config, BASH_PERMISSION_MIGRATION)
if (existsSync(marker)) return
const done = () =>
Bun.write(marker, "").then(
() => undefined,
(err) => log.warn("failed to record bash permission migration", { path: marker, err }),
)
const files = GLOBAL_CONFIG_FILES.map((f) => path.join(Global.Path.config, f))
const legacy = path.join(Global.Path.config, "config")
const existing = files.filter((f) => existsSync(f))
const hasLegacy = existsSync(legacy)

// no global config → new user, they'll get the new bash:ask default
if (existing.length === 0 && !hasLegacy) return
if (existing.length === 0 && !hasLegacy) return done()

const configs: Array<{ file: string; data: Record<string, unknown> }> = []
// check if any config file already has an explicit bash permission
Expand All @@ -430,17 +439,17 @@ export namespace KilocodeConfig {
.catch(() => "")
const data = parseJsonc(text) ?? {}
configs.push({ file, data })
if (typeof data.permission === "string" || (isRecord(data.permission) && data.permission.bash)) return
if (typeof data.permission === "string" || (isRecord(data.permission) && data.permission.bash)) return done()
}

// A schema-only file is generated for editor completion. It does not mean
// the user predates the bash permission default.
if (!hasLegacy && configs.every((item) => Object.keys(item.data).every((key) => key === "$schema"))) return
if (!hasLegacy && configs.every((item) => Object.keys(item.data).every((key) => key === "$schema"))) return done()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After these changes the migration will be marked as done even in cases of unreadable or malformed config. Fixing the config won't retry the migration. Can you address this issue? Potentially distinguish read/parse failures from empty valid configs and add regression tests to cover these scenarios.


// also check legacy TOML config for bash permission
if (hasLegacy) {
const toml = await import(pathToFileURL(legacy).href, { with: { type: "toml" } }).catch(() => undefined)
if (toml?.default?.permission?.bash) return
if (toml?.default?.permission?.bash) return done()
}

// existing user without bash permission → write bash:allow to highest-precedence file
Expand All @@ -454,13 +463,15 @@ export namespace KilocodeConfig {
formattingOptions: { insertSpaces: true, tabSize: 2 },
})
await Bun.write(target, applyEdits(text, edits))
await done()
log.info("migrated bash permission to allow for existing user", { path: target })
return
}

const data = parseJsonc(text) ?? {}
const merged = { ...data, permission: { ...data.permission, bash: "allow" } }
await Bun.write(target, JSON.stringify(merged, null, 2))
await done()
log.info("migrated bash permission to allow for existing user", { path: target })
}

Expand Down
51 changes: 51 additions & 0 deletions packages/opencode/test/kilocode/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,4 +1034,55 @@ describe("bash permission migration", () => {
await disposeAllInstances()
}
})

test("does not restore a migrated bash permission after the user deletes it", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
await writeConfig(dir, { permission: { read: "allow" } }, "kilo.jsonc")
},
})

const prev = Global.Path.config
;(Global.Path as { config: string }).config = tmp.path
await clear()
await disposeAllInstances()

try {
const file = path.join(tmp.path, "kilo.jsonc")
await KilocodeConfig.migrateBashPermission()
expect(JSON.parse(await Filesystem.readText(file)).permission.bash).toBe("allow")

await writeConfig(tmp.path, { permission: { read: "allow" } }, "kilo.jsonc")
await KilocodeConfig.migrateBashPermission()

expect(JSON.parse(await Filesystem.readText(file)).permission).toEqual({ read: "allow" })
} finally {
;(Global.Path as { config: string }).config = prev
await clear()
await disposeAllInstances()
}
})

test("does not later migrate a fresh install after its config gains settings", async () => {
await using tmp = await tmpdir()

const prev = Global.Path.config
;(Global.Path as { config: string }).config = tmp.path
await clear()
await disposeAllInstances()

try {
await KilocodeConfig.migrateBashPermission()
await writeConfig(tmp.path, { model: "test/model" }, "kilo.jsonc")
await KilocodeConfig.migrateBashPermission()

expect(JSON.parse(await Filesystem.readText(path.join(tmp.path, "kilo.jsonc")))).toEqual({
model: "test/model",
})
} finally {
;(Global.Path as { config: string }).config = prev
await clear()
await disposeAllInstances()
}
})
})
Loading