Skip to content

fix(apps): forbid online env var delete; surface release note #2844

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.

Sign up for GitHub

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

Open
chengshihao-miaoda wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(apps): forbid online env var delete; surface release note #2844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions shortcuts/apps/apps_env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (
const (
defaultAppsEnvVarEnv = "dev"
defaultAppsEnvVarScene = 2
// envVarReleaseNote tells users that mutations only propagate to the online
// runtime after a release — the config write succeeds immediately but the
// running app keeps serving the previously released snapshot until then.
envVarReleaseNote = "changes take effect online only after a release; run `lark-cli apps +release-create --app-id APP_ID`"
)

// AppsEnvVarList lists app environment variables without values by default.
Expand Down Expand Up @@ -148,6 +152,7 @@ var AppsEnvVarSet = common.Shortcut{
"key": key,
"env": env,
"action": action,
"note": envVarReleaseNote,
}, nil, nil)
return nil
},
Expand Down Expand Up @@ -177,6 +182,10 @@ var AppsEnvVarDelete = common.Shortcut{
if err := validateEnvVarEnv(envVarEnv(rctx)); err != nil {
return err
}
if envVarEnv(rctx) == "online" {
return appsFailedPreconditionParamError("--environment",
"deleting online env vars via CLI is disabled; delete the key in dev with --environment dev and run `lark-cli apps +release-create --app-id <app_id>` to apply, or remove it from the developer console")
}
_, err := requireEnvVarKeys(rctx.StrArray("key"))
return err
},
Expand Down Expand Up @@ -209,6 +218,7 @@ var AppsEnvVarDelete = common.Shortcut{
rctx.OutFormat(map[string]interface{}{
"env": env,
"deleted_keys": deletedKeys,
"note": envVarReleaseNote,
}, nil, nil)
return nil
},
Expand Down
59 changes: 33 additions & 26 deletions shortcuts/apps/apps_env_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ func TestAppsEnvVarSet_ExecutesWithYesAndDoesNotEchoValue(t *testing.T) {
t.Fatalf("stdout missing %q: %s", want, got)
}
}
if !strings.Contains(got, envVarReleaseNote) {
t.Fatalf("stdout missing release note %q: %s", envVarReleaseNote, got)
}
}

func TestAppsEnvVarDelete_IsHighRiskWrite(t *testing.T) {
Expand All @@ -282,7 +285,7 @@ func TestAppsEnvVarDelete_BuildsDeleteBodyWithKeys(t *testing.T) {
reg.Register(stub)

if err := runAppsShortcut(t, AppsEnvVarDelete,
[]string{"+env-delete", "--app-id", "app_x", "--environment", "online",
[]string{"+env-delete", "--app-id", "app_x", "--environment", "dev",
"--key", "SECRET_ONE", "--key", "SECRET_TWO", "--yes", "--as", "user"}, factory, stdout); err != nil {
t.Fatalf("execute err=%v", err)
}
Expand All @@ -291,19 +294,22 @@ func TestAppsEnvVarDelete_BuildsDeleteBodyWithKeys(t *testing.T) {
if err := json.Unmarshal(stub.CapturedBody, &sent); err != nil {
t.Fatalf("decode body: %v", err)
}
if sent["env"] != "online" {
t.Fatalf("body.env = %v, want online", sent["env"])
if sent["env"] != "dev" {
t.Fatalf("body.env = %v, want dev", sent["env"])
}
keys, ok := sent["keys"].([]interface{})
if !ok || len(keys) != 2 || keys[0] != "SECRET_ONE" || keys[1] != "SECRET_TWO" {
t.Fatalf("body.keys = %#v, want SECRET_ONE/SECRET_TWO", sent["keys"])
}
got := stdout.String()
for _, want := range []string{`"env": "online"`, `"deleted_keys"`, `"SECRET_ONE"`, `"SECRET_TWO"`} {
for _, want := range []string{`"env": "dev"`, `"deleted_keys"`, `"SECRET_ONE"`, `"SECRET_TWO"`} {
if !strings.Contains(got, want) {
t.Fatalf("stdout missing %q: %s", want, got)
}
}
if !strings.Contains(got, envVarReleaseNote) {
t.Fatalf("stdout missing release note %q: %s", envVarReleaseNote, got)
}
}

func TestAppsEnvVarDelete_NotModifiableHint(t *testing.T) {
Expand Down Expand Up @@ -337,28 +343,29 @@ func TestAppsEnvVarDelete_NotModifiableHint(t *testing.T) {
}
}

func TestAppsEnvVarDelete_OnlineDryRunDoesNotRequireYes(t *testing.T) {
factory, stdout, _ := newAppsExecuteFactory(t)
if err := runAppsShortcut(t, AppsEnvVarDelete,
[]string{"+env-delete", "--app-id", "app_x", "--environment", "online",
"--key", "SECRET_ONE", "--key", "SECRET_TWO", "--dry-run", "--as", "user"}, factory, stdout); err != nil {
t.Fatalf("dry-run err=%v", err)
}

var dryRun dryRunAPIEnvelope
got := stdout.String()
if err := json.Unmarshal([]byte(got), &dryRun); err != nil {
t.Fatalf("decode dry-run: %v\n%s", err, got)
}
if len(dryRun.API) != 1 || dryRun.API[0].Method != "POST" || dryRun.API[0].URL != "/open-apis/spark/v1/apps/app_x/delete_env_vars" {
t.Fatalf("dry-run api = %#v", dryRun.API)
}
if dryRun.API[0].Body["env"] != "online" {
t.Fatalf("dry-run body.env = %v, want online", dryRun.API[0].Body["env"])
}
keys, ok := dryRun.API[0].Body["keys"].([]interface{})
if !ok || len(keys) != 2 || keys[0] != "SECRET_ONE" || keys[1] != "SECRET_TWO" {
t.Fatalf("dry-run body.keys = %#v, want SECRET_ONE/SECRET_TWO", dryRun.API[0].Body["keys"])
func TestAppsEnvVarDelete_RejectsOnlineEnvironment(t *testing.T) {
for _, args := range [][]string{
{"+env-delete", "--app-id", "app_x", "--environment", "online",
"--key", "SECRET_ONE", "--yes", "--as", "user"},
{"+env-delete", "--app-id", "app_x", "--environment", "online",
"--key", "SECRET_ONE", "--dry-run", "--as", "user"},
} {
factory, stdout, _ := newAppsExecuteFactory(t)
err := runAppsShortcut(t, AppsEnvVarDelete, args, factory, stdout)
requireAppsProblem(t, err, errs.CategoryValidation)
var validation *errs.ValidationError
if !errors.As(err, &validation) {
t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err)
}
if validation.Param != "--environment" {
t.Fatalf("validation param = %q, want --environment", validation.Param)
}
if validation.Subtype != errs.SubtypeFailedPrecondition {
t.Fatalf("validation subtype = %q, want %q", validation.Subtype, errs.SubtypeFailedPrecondition)
}
if !strings.Contains(err.Error(), "disabled") {
t.Fatalf("error should explain online delete is disabled: %v", err)
}
}
}

Expand Down
7 changes: 5 additions & 2 deletions skills/lark-apps/references/lark-apps-env.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ lark-cli apps +env-list --app-id <app_id> --include-values --jq '.data.items[] |

dev 环境设置不需要 `--yes`。设置 online 环境需要人类确认并显式传 `--yes`；如果用户在同一轮已经明确说“确认/直接执行”，视为已确认，直接带 `--yes`，不要再次追问。`--dry-run` 可用于预览请求且不需要 `--yes`。变量值支持直接传 `<value>`，也支持 `@file` 或 stdin 输入。

成功响应会带 `data.note` 提示变更需要走 `+release-create` 才会在线上生效，转述给用户即可。

回复中只说明 app/env/key 和执行结果；不要回显真实 value。需要举例时使用 `<value>`、`@file` 或 stdin。

```bash
Expand All @@ -33,12 +35,13 @@ lark-cli apps +env-set --app-id <app_id> --environment online --key FOO --value

## 删除

`+env-delete` 是 high-risk-write。尊重 exit 10 confirmation protocol：先让用户确认 app/env/key 和删除后果，再传 `--yes`。不要自动补 `--yes`。如果只是认证失败后让用户重登，重登完成不等于删除确认；继续删除前仍需确认。
`+env-delete` 是 high-risk-write，且 **仅支持 dev 环境**。CLI 禁止删除 online 环境变量——传 `--environment online` 会被 Validate 拒绝（failed_precondition）；若用户要清理 online 的 key，让其先在 dev 中删除，再 `+release-create` 推上线，或到开发者后台删除。

尊重 exit 10 confirmation protocol：先让用户确认 app/env/key 和删除后果，再传 `--yes`。不要自动补 `--yes`。如果只是认证失败后让用户重登，重登完成不等于删除确认；继续删除前仍需确认。成功响应同样会带 `data.note`，提醒删除需要发布后在线上生效。

```bash
lark-cli apps +env-delete --app-id <app_id> --key FOO --dry-run
lark-cli apps +env-delete --app-id <app_id> --key FOO --yes
lark-cli apps +env-delete --app-id <app_id> --environment online --key FOO --yes
```

## 反模式
Expand Down
Loading