mirror of
https://github.com/TencentCloud/Octop.git
synced 2026-10-21 06:29:24 +08:00
* fix(config): never rewrite an unparseable config.json as empty (#730)
Four call sites read config.json, treated a parse failure as an empty dict and
wrote that back, destroying every other key. One trailing comma plus one
`octop run --port 8088` wiped 17 of 18 keys including the whole `database`
section; the next boot silently came up as a greenfield SQLite instance (fresh
setup-wizard password, zero warnings) while the PostgreSQL data sat unreachable.
Losing `bind_host` the same way is what makes #730's reporter unable to reach
the server from the LAN.
A torn write creates the same precondition with no user error: three of the four
sites wrote config.json with a bare write_text, so a crash / OOM / full disk
mid-write left invalid JSON that the next plugin toggle then wiped.
One shared policy in a new leaf helper, infra/utils/json_file.py:
absent file -> {} and write (first run, unchanged)
valid JSON object -> merge and write (unchanged)
unparseable/non-object -> raise CONFIG_FILE_CORRUPT, write nothing
The error names the file and the parser line/column but never the contents,
which hold database credentials. Writes now go through write_json_atomic (temp +
os.replace, fd closed on every path, temp removed on failure) so no reader can
observe a torn file.
Sites: cli/commands/run.py (_save_configfile_overrides, plus
_load_configfile_overrides which used to fall back to launch defaults),
infra/agents/plugins/manager.py (_write_global_plugin_enabled = the dashboard
toggle), infra/setup/service.py (persist_bind_options),
infra/agents/plugins/seed.py. `octop run` / `octop service start` report the
error and exit 1 via fail_octop (now NoReturn) instead of a traceback.
Supersedes the pinned "treat a malformed file as empty so the next run rewrites
it cleanly" contract: its fixture (`{not valid json`) had nothing to lose, so
the test could not observe the destruction. Rewritten as
test_run_reports_corrupt_config_cleanly and
test_run_does_not_overwrite_corrupt_config.
Verified end-to-end against a real OctopServer + real PostgreSQL. Before: one
`octop run --port` flipped driver postgresql -> sqlite, the instance reverted to
the first-run wizard, and users=1/agents=1 were orphaned in PG with no warning.
After: exit 1, config.json byte-identical, boot fails loudly instead of
switching backends. Mutation check: restoring the lenient policy in the helper
reddens 9 tests spanning all four sites.
make all green (3299 passed, 16 skipped); mypy --strict clean; dashboard
`npx tsc --noEmit` clean.
* fix(config): name the file when config.json is corrupt (#730)
`load_config` let a bare `json.JSONDecodeError` escape: it reports a line and
column but never says *which* file failed, and this entry point is reached from
boot, backup and several CLI commands — not only `octop run`. Raise a ValueError
naming the path and the parser position, refuse a non-object document, and never
echo file contents (config.json holds database credentials).
Same policy as infra/utils/json_file.py, duplicated here because octop.config
must stay free of infra imports (AGENTS.md §5).
`octop run` already exits cleanly on a corrupt config through resolve_bind; this
covers the remaining entry points.
make all green (3301 passed, 16 skipped); mypy --strict clean.
* fix(config): preserve config.json permissions in atomic writes
Self-review against CONTRIBUTING.md / AGENTS.md §7 caught a regression in the
atomic writer added earlier in this branch: tempfile.mkstemp creates the temp
file 0600, so replacing config.json silently tightened an existing 0644 file to
0600. Under `octop service start --scope system` that runs as root and writes
into the service user's home, a root-owned 0600 config.json is unreadable by the
service user and the unit cannot start; the bare write_text it replaced produced
the umask default.
write_json_atomic now chmods the temp file to the mode of the file being
replaced, or to `0o666 & ~umask` for a new file — matching the two pre-existing
config.json writers (infra/db/rebind.py, infra/backup/auto.py; both
write_text + replace) rather than mkstemp's 0600. POSIX-guarded, so Windows
behaviour is unchanged and no test asserts mode bits off POSIX (AGENTS.md §7).
Also adds the missing CHANGELOG line for the load_config error-message change.
Verified: new file -> 0644, existing 0644 -> 0644, existing 0600 -> 0600.
make all green (3303 passed, 16 skipped); make typecheck-frontend green;
eslint 0 errors (67 pre-existing react-hooks warnings).
Note: `make check-all` is red on develop itself — prettier --check fails on
dashboard/src/pages/Agent/Skills/components/SkillsTabs.tsx exactly as committed
by 83378876. Left untouched here; confirmed pre-existing by checking develop's
version of that file in isolation.
106 lines
3.8 KiB
Python
106 lines
3.8 KiB
Python
"""tests/unit/i18n/test_errors.py"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from octop.i18n import error_message
|
|
from octop.infra.errors import ErrorCode, OctopError
|
|
from octop.infra.utils.locale import resolve_request_locale
|
|
|
|
|
|
def test_every_error_code_has_i18n_entry():
|
|
for code in ErrorCode:
|
|
assert error_message(code.value, "en")
|
|
assert error_message(code.value, "zh")
|
|
|
|
|
|
def test_error_message_zh():
|
|
assert "名称" in error_message("AGENT_NAME_TAKEN", "zh")
|
|
|
|
|
|
def test_octop_error_localized_factory():
|
|
err = OctopError.localized(ErrorCode.FORBIDDEN, "zh")
|
|
assert err.code is ErrorCode.FORBIDDEN
|
|
assert err.message == "没有权限。"
|
|
|
|
|
|
def test_octop_error_to_envelope_with_locale():
|
|
err = OctopError(ErrorCode.AGENT_NAME_TAKEN, "agent name 'x' already in use")
|
|
envelope = err.to_envelope(locale="zh")
|
|
assert envelope["error"]["code"] == "AGENT_NAME_TAKEN"
|
|
assert envelope["error"]["message"] == "该名称已被使用,请换一个名称。"
|
|
|
|
|
|
def test_resolve_request_locale_from_accept_language():
|
|
class _Headers:
|
|
def get(self, key: str) -> str | None:
|
|
if key.lower() == "accept-language":
|
|
return "zh-CN,en;q=0.9"
|
|
return None
|
|
|
|
class _Req:
|
|
headers = _Headers()
|
|
|
|
assert resolve_request_locale(_Req()) == "zh"
|
|
|
|
|
|
def test_dashboard_api_errors_match_backend():
|
|
repo = Path(__file__).resolve().parents[3]
|
|
dash_en = json.loads((repo / "dashboard/src/locales/en.json").read_text(encoding="utf-8"))
|
|
backend_en = json.loads((repo / "src/octop/i18n/en.json").read_text(encoding="utf-8"))
|
|
dash_codes = set(dash_en["apiErrors"].keys())
|
|
backend_codes = set(backend_en["errors"].keys())
|
|
assert dash_codes == backend_codes == {c.value for c in ErrorCode}
|
|
|
|
|
|
# i18next uses ``{{name}}``; a lone ``{name}`` is left uninterpolated in the UI.
|
|
_DASHBOARD_SINGLE_BRACE = re.compile(r"(?<!\{)\{([a-zA-Z_][a-zA-Z0-9_]*)\}(?!\})")
|
|
|
|
|
|
def test_dashboard_api_errors_use_i18next_placeholders():
|
|
repo = Path(__file__).resolve().parents[3]
|
|
for locale in ("en", "zh"):
|
|
data = json.loads(
|
|
(repo / f"dashboard/src/locales/{locale}.json").read_text(encoding="utf-8")
|
|
)
|
|
for code, msg in data["apiErrors"].items():
|
|
found = _DASHBOARD_SINGLE_BRACE.findall(msg)
|
|
assert not found, (
|
|
f"{locale} apiErrors.{code} uses Python-style {{{', '.join(found)}}} — "
|
|
"dashboard i18next needs {{name}} double braces"
|
|
)
|
|
|
|
|
|
def test_login_locked_interpolates_minutes():
|
|
assert "15" in error_message("LOGIN_LOCKED", "zh", minutes=15)
|
|
assert "minutes" not in error_message("LOGIN_LOCKED", "zh", minutes=15).lower()
|
|
assert "{minutes}" not in error_message("LOGIN_LOCKED", "en", minutes=15)
|
|
|
|
|
|
def test_knowledge_doc_too_large_interpolates_max_mb():
|
|
assert "100" in error_message("KNOWLEDGE_DOC_TOO_LARGE", "zh", max_mb=100)
|
|
assert "{max_mb}" not in error_message("KNOWLEDGE_DOC_TOO_LARGE", "en", max_mb=100)
|
|
|
|
|
|
def test_localized_message_falls_back_when_key_missing(monkeypatch: pytest.MonkeyPatch):
|
|
err = OctopError(ErrorCode.AUTH_FAILED, "custom detail")
|
|
monkeypatch.setattr(
|
|
"octop.infra.errors.i18n_error_message",
|
|
lambda *_a, **_k: (_ for _ in ()).throw(KeyError("errors.X")),
|
|
)
|
|
assert err.localized_message("zh") == "custom detail"
|
|
|
|
|
|
def test_config_file_corrupt_interpolates_path_and_detail():
|
|
kwargs = {"path": "~/.octop/config.json", "detail": "line 3, column 1"}
|
|
assert "~/.octop/config.json" in error_message("CONFIG_FILE_CORRUPT", "en", **kwargs)
|
|
assert "line 3, column 1" in error_message("CONFIG_FILE_CORRUPT", "zh", **kwargs)
|
|
for locale in ("en", "zh"):
|
|
msg = error_message("CONFIG_FILE_CORRUPT", locale, **kwargs)
|
|
assert "{path}" not in msg and "{detail}" not in msg
|