fix: a version write keeps the document it lands in and the mode it had

Inserting `version` above a leading `---` put it in a document of its own, so
every setting after the marker parsed as a second document and was silently
ignored: the installation came up on defaults with a config that looked intact.
Both writers - the Go one and upgrade.sh's - now insert after the document
start, and the parse-back check still refuses anything a line rewrite cannot do
safely.

upgrade.sh rewrote config.yaml through a temp file created with the process
umask, so a 0600 config came back 0644 on every package upgrade - the file
holding the API keys became group/world readable. It now creates the temp file
privately and restores the original mode before the atomic replace.

A package upgrade also deleted `.update-build-pending`, because rsync --delete
only keeps what the exclude list names: the installation forgot a compile it
still owed and the next click reported "already up to date". Every one-click
update artifact is on the keep list now, asserted against .gitignore so the
next one cannot be forgotten the same way.
This commit is contained in:
Sycun committed 2026-10-08 22:46:29 +08:00
1 parent b645914e1f
commit 36e1d9680b
4 files changed
+245 -12

No files matched your search

+143
View File
@@ -2,9 +2,13 @@ package main
import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
"gopkg.in/yaml.v3"
)
// Every `go build` in this repository's scripts has to target the cmd/server package.
@@ -45,3 +49,142 @@ func TestScriptsBuildThePackageNotASingleFile(t *testing.T) {
t.Fatal("no `go build` command found in the scripts")
}
}
func TestUpgradeScriptUpdateConfigVersionPreservesDocumentAndMode(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("upgrade.sh requires a POSIX shell")
}
bash, err := exec.LookPath("bash")
if err != nil {
t.Skip("bash is not available")
}
if _, err := exec.LookPath("python3"); err != nil {
t.Skip("python3 is not available")
}
root := filepath.Join("..", "..")
scriptPath := filepath.Join(root, "upgrade.sh")
script, err := os.ReadFile(scriptPath)
if err != nil {
t.Fatalf("read %s: %v", scriptPath, err)
}
function := extractShellFunction(t, string(script), "update_config_version")
command := "umask 022\n" + function + "\nCONFIG_FILE=$1\nupdate_config_version \"$2\"\n"
for _, tc := range []struct {
name string
original string
wantDocumentStart bool
}{
{name: "document start", original: "---\nserver:\n port: 8088\n", wantDocumentStart: true},
{name: "private mode", original: "version: \"v1.0.0\"\nserver:\n port: 8088\n"},
} {
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()
configPath := filepath.Join(dir, "config.yaml")
if err := os.WriteFile(configPath, []byte(tc.original), 0o600); err != nil {
t.Fatal(err)
}
if err := os.Chmod(configPath, 0o600); err != nil {
t.Fatal(err)
}
cmd := exec.Command(bash, "-c", command, "upgrade-version-test", configPath, "v1.2.3")
if output, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("update_config_version failed: %v\n%s", err, output)
}
got, err := os.ReadFile(configPath)
if err != nil {
t.Fatal(err)
}
if tc.wantDocumentStart && !strings.HasPrefix(string(got), "---\n") {
t.Errorf("YAML document start must remain first: %q", got)
}
var parsed struct {
Version string `yaml:"version"`
Server struct {
Port int `yaml:"port"`
} `yaml:"server"`
}
if err := yaml.Unmarshal(got, &parsed); err != nil {
t.Fatalf("updated config is not valid YAML: %v\n%s", err, got)
}
if parsed.Version != "v1.2.3" {
t.Errorf("version = %q, want v1.2.3", parsed.Version)
}
if parsed.Server.Port != 8088 {
t.Errorf("server.port = %d, want 8088", parsed.Server.Port)
}
info, err := os.Stat(configPath)
if err != nil {
t.Fatal(err)
}
if info.Mode().Perm() != 0o600 {
t.Errorf("mode = %v, want 0600 preserved", info.Mode().Perm())
}
})
}
}
// The tarball path syncs with `rsync --delete`, so every artifact a one-click update writes
// at the install root has to be on its keep list. .gitignore is exactly that list of
// machine-local artifacts, so the assertion is derived from it: a new marker cannot be
// forgotten here and silently deleted by the next package upgrade.
func TestUpgradeScriptKeepsEveryOneClickUpdateArtifact(t *testing.T) {
root := filepath.Join("..", "..")
ignore, err := os.ReadFile(filepath.Join(root, ".gitignore"))
if err != nil {
t.Fatal(err)
}
script, err := os.ReadFile(filepath.Join(root, "upgrade.sh"))
if err != nil {
t.Fatal(err)
}
var artifacts []string
inSection := false
for _, line := range strings.Split(string(ignore), "\n") {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "# One-click update artifacts") {
inSection = true
continue
}
if !inSection {
continue
}
if strings.HasPrefix(line, "#") {
break
}
if line != "" {
artifacts = append(artifacts, line)
}
}
if len(artifacts) == 0 {
t.Fatal("no update artifacts found in .gitignore: the section this test reads has moved")
}
// The script names the binary through a variable; compare against the literal name.
keeps := strings.ReplaceAll(string(script), "${BINARY_NAME}", "cyberstrike-ai")
for _, artifact := range artifacts {
if !strings.Contains(keeps, "--exclude="+artifact) {
t.Errorf("upgrade.sh syncs with rsync --delete but does not keep %s", artifact)
}
}
}
func extractShellFunction(t *testing.T, script, name string) string {
t.Helper()
startMarker := name + "() {"
start := strings.Index(script, startMarker)
if start < 0 {
t.Fatalf("function %s not found", name)
}
lines := strings.Split(script[start:], "\n")
for i := 1; i < len(lines); i++ {
if lines[i] == "}" {
return strings.Join(lines[:i+1], "\n")
}
}
t.Fatalf("function %s has no closing brace", name)
return ""
}
+24 -3
View File
@@ -1796,9 +1796,21 @@ func topVersionKey(line string) bool {
return false
}
func yamlDocumentStart(line string) bool {
line = strings.TrimPrefix(strings.TrimSuffix(line, "\r"), "\uFEFF")
if !strings.HasPrefix(line, "---") {
return false
}
rest := line[len("---"):]
if strings.Trim(rest, " \t") == "" {
return true
}
return len(rest) > 0 && (rest[0] == ' ' || rest[0] == '\t') && strings.HasPrefix(strings.TrimLeft(rest, " \t"), "#")
}
// WriteVersion 就地替换配置文件的顶层 version 字段,保留其它每一行的原样(tarball 升级
// 路径一直这么做);没有这个字段就插到文件开头。返回是否真的改了。写盘走临时文件 + 原子
// rename——这是运维者的活配置,不能留半截。替换后先把新文本解析回验一遍:行级替换够不着的
// 路径一直这么做);没有这个字段就插到文件开头(若文件以 YAML 文档标记开头,则插在标记后)。
// 返回是否真的改了。写盘走临时文件 + 原子 rename——这是运维者的活配置,不能留半截。替换后先把新文本解析回验一遍:行级替换够不着的
// 写法(JSON 单行、多行标量……)解析失败或读不回目标值就拒绝写、原文件不动。版本号停在旧值
// 只是页头显示问题,写坏配置是起不来。
func WriteVersion(path, version string) (bool, error) {
@@ -1827,7 +1839,12 @@ func WriteVersion(path, version string) (bool, error) {
}
}
if !replaced {
lines = append([]string{`version: "` + version + `"`}, lines...)
versionLine := `version: "` + version + `"`
if len(lines) > 0 && yamlDocumentStart(lines[0]) {
lines = append([]string{lines[0], versionLine}, lines[1:]...)
} else {
lines = append([]string{versionLine}, lines...)
}
}
out := strings.Join(lines, "\n")
if out == string(data) {
@@ -1840,6 +1857,10 @@ func WriteVersion(path, version string) (bool, error) {
if err := os.WriteFile(tmp, []byte(out), info.Mode().Perm()); err != nil {
return false, err
}
if err := os.Chmod(tmp, info.Mode().Perm()); err != nil {
_ = os.Remove(tmp)
return false, err
}
if err := os.Rename(tmp, path); err != nil {
return false, err
}
+43
View File
@@ -385,6 +385,9 @@ func TestFileVersionAndWriteVersion(t *testing.T) {
if err := os.WriteFile(path, []byte(original), 0o640); err != nil {
t.Fatal(err)
}
if err := os.Chmod(path, 0o640); err != nil {
t.Fatal(err)
}
if v, err := FileVersion(path); err != nil || v != "v1.0.0" {
t.Fatalf("version = %q err = %v", v, err)
}
@@ -425,6 +428,46 @@ func TestFileVersionAndWriteVersion(t *testing.T) {
}
}
func TestWriteVersionPreservesYAMLDocumentStart(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "config.yaml")
original := "---\nserver:\n port: 8088\n"
const originalMode = os.FileMode(0o600)
if err := os.WriteFile(path, []byte(original), originalMode); err != nil {
t.Fatal(err)
}
changed, err := WriteVersion(path, "v1.2.3")
if err != nil || !changed {
t.Fatalf("WriteVersion: changed=%v err=%v", changed, err)
}
got := readTestFile(t, path)
want := "---\nversion: \"v1.2.3\"\nserver:\n port: 8088\n"
if got != want {
t.Fatalf("file after write:\n%q\nwant:\n%q", got, want)
}
if !strings.HasPrefix(got, "---\n") {
t.Fatalf("YAML document start must remain first: %q", got)
}
if version, err := FileVersion(path); err != nil || version != "v1.2.3" {
t.Fatalf("FileVersion = %q, %v; want v1.2.3", version, err)
}
cfg, err := Load(path)
if err != nil {
t.Fatalf("Load after WriteVersion: %v", err)
}
if cfg.Server.Port != 8088 {
t.Fatalf("server.port = %d, want 8088", cfg.Server.Port)
}
info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if info.Mode().Perm() != originalMode {
t.Fatalf("mode = %v, want %v preserved", info.Mode().Perm(), originalMode)
}
}
// FileVersion 用 YAML 解析(引号键认得出),WriteVersion 若只认顶格裸键,合法配置就会被追加
// 出第二个 version 键、整个文件随后 Load 失败。这条测试钉死两者口径一致,以及"改不了就拒绝写"。
func TestWriteVersionRewritesQuotedKeysAndRefusesWhatItCannot(t *testing.T) {
+35 -9
View File
@@ -28,7 +28,7 @@ set -euo pipefail
# - tools/ (user extensions; never overwritten by upgrade)
# - roles/, skills/, agents/
# - the rollback points of an earlier one-click update:
# .update-backup/, .update-state.json, cyberstrike-ai.prev
# .update-backup/, .update-state.json, .update-build-pending, cyberstrike-ai.prev
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
cd "$ROOT_DIR"
@@ -285,7 +285,7 @@ confirm_or_exit() {
fi
info " - Preserve tools/: yes (always)"
info " - Preserve roles/skills/agents: yes (always)"
info " - Preserve rollback points (.update-backup/, .update-state.json, ${BINARY_NAME}.prev): yes"
info " - Preserve rollback points (.update-backup/, .update-state.json, .update-build-pending, ${BINARY_NAME}.prev): yes"
info " - Stop service: ${STOP_SERVICE}"
fi
@@ -511,7 +511,7 @@ update_config_version() {
# config that no longer loads is an installation that no longer starts.
local new_tag="$1"
python3 - "$CONFIG_FILE" "$new_tag" <<'PY'
import os, re, sys
import os, re, stat, sys, tempfile
path = sys.argv[1]
tag = sys.argv[2]
@@ -544,11 +544,33 @@ for i, line in enumerate(lines):
if not refused:
if not replaced:
out.insert(0, 'version: "%s"\n' % tag)
tmp = path + ".tmp"
with open(tmp, "w", encoding="utf-8") as f:
f.writelines(out)
os.replace(tmp, path)
insert_at = 0
if lines:
first = lines[0].rstrip("\r\n")
if first.startswith("\ufeff"):
first = first[1:]
rest = first[3:] if first.startswith("---") else None
if rest is not None and (not rest.strip(" \t") or rest[:1] in (" ", "\t") and rest.lstrip(" \t").startswith("#")):
insert_at = 1
if not lines[0].endswith(("\n", "\r")):
lines[0] += "\n"
out[0] = lines[0]
out.insert(insert_at, 'version: "%s"\n' % tag)
original_mode = stat.S_IMODE(os.stat(path).st_mode)
tmp_fd, tmp = tempfile.mkstemp(prefix=os.path.basename(path) + ".", suffix=".tmp", dir=os.path.dirname(os.path.abspath(path)))
try:
with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
f.writelines(out)
f.flush()
os.fchmod(f.fileno(), original_mode)
os.replace(tmp, path)
except BaseException:
try:
os.unlink(tmp)
except FileNotFoundError:
pass
raise
PY
}
@@ -588,10 +610,14 @@ sync_code() {
rsync_excludes+=( "--exclude=agents/" )
# Rollback points written by a one-click update: an rsync --delete that ate them would
# leave the installation with no way back.
# leave the installation with no way back - and eating .update-build-pending would make it
# forget a compile it still owes, so the next click reports "already up to date".
rsync_excludes+=( "--exclude=.update-backup/" )
rsync_excludes+=( "--exclude=.update-staging/" )
rsync_excludes+=( "--exclude=.update-state.json" )
rsync_excludes+=( "--exclude=.update-state.json.tmp" )
rsync_excludes+=( "--exclude=.update-build-pending" )
rsync_excludes+=( "--exclude=.update-build-pending.tmp" )
rsync_excludes+=( "--exclude=${BINARY_NAME}.prev" )
# Ensure this upgrade script itself is not deleted.