Skip to content

Commit b795812

Browse files
committed
fix(cli): refuse to overwrite corrupt config.json on gitt config set (entrius#845)
When `~/.gittensor/config.json` failed to parse, `config_set` printed a yellow warning, set `config = {}`, then wrote a fresh single-key file — silently destroying every previously-configured key (network, contract_address, ws_endpoint, hotkey, ...). Mirror PR entrius#817's read-side fix: `JSONDecodeError` now aborts with a clear message and `SystemExit(1)`, leaving the corrupt file untouched so the operator can inspect or recover it. New regression tests in `tests/cli/test_config_set.py` cover the abort-with-nonzero-exit, byte-for-byte preservation of the corrupt file, non-regression of the valid-file merge path, and the first-run case. Closes entrius#845
1 parent 345a523 commit b795812

2 files changed

Lines changed: 104 additions & 3 deletions

File tree

‎gittensor/cli/main.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,23 @@ def config_set(key: str, value: str):
117117
# Ensure config directory exists
118118
GITTENSOR_DIR.mkdir(parents=True, exist_ok=True)
119119

120-
# Load existing config or start fresh
120+
# Load existing config or start fresh.
121+
# On JSONDecodeError refuse to overwrite — the file may contain values the
122+
# operator still relies on (network, contract_address, ws_endpoint, hotkey).
123+
# Silently writing a fresh single-key config would erase those and fall
124+
# through to mainnet defaults on the next read. Mirrors load_config's
125+
# read-side fix (PR #817 / #816).
121126
config = {}
122127
if CONFIG_FILE.exists():
123128
try:
124129
config = json.loads(CONFIG_FILE.read_text())
125-
except json.JSONDecodeError:
126-
console.print('[yellow]Warning: Existing config was invalid, starting fresh[/yellow]')
130+
except json.JSONDecodeError as e:
131+
console.print(
132+
f'[red]Error: Config file at {CONFIG_FILE} is not valid JSON ({e}).[/red]\n'
133+
f'[red]Refusing to overwrite — inspect or move the file aside before retrying[/red]\n'
134+
f'[red]so previously-configured keys (network, contract_address, ws_endpoint, ...) are not lost.[/red]'
135+
)
136+
raise SystemExit(1)
127137

128138
# Set the value
129139
old_value = config.get(key)

‎tests/cli/test_config_set.py‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# The MIT License (MIT)
2+
# Copyright © 2025 Entrius
3+
4+
"""
5+
Tests for `gitt config set` corrupt-file handling.
6+
7+
Regression: a single corrupt-config recovery used to wipe every previously-set
8+
key and replace the file with `{key: value}`. Operator's `network`,
9+
`contract_address`, `ws_endpoint`, and `hotkey` would silently disappear and
10+
the next `gitt issues ...` would fall through to finney mainnet. Companion to
11+
PR #817 / issue #816 (read side) and closes issue #845 (write side).
12+
"""
13+
14+
import json
15+
from pathlib import Path
16+
from unittest.mock import patch
17+
18+
import pytest
19+
from click.testing import CliRunner
20+
21+
from gittensor.cli.main import config_group
22+
23+
24+
@pytest.fixture
25+
def temp_config_dir(tmp_path: Path):
26+
"""Redirect CONFIG_FILE/GITTENSOR_DIR to a temp dir for one test."""
27+
config_dir = tmp_path / '.gittensor'
28+
config_file = config_dir / 'config.json'
29+
with (
30+
patch('gittensor.cli.main.GITTENSOR_DIR', config_dir),
31+
patch('gittensor.cli.main.CONFIG_FILE', config_file),
32+
):
33+
yield config_dir, config_file
34+
35+
36+
def _read(config_file: Path) -> dict:
37+
return json.loads(config_file.read_text()) if config_file.exists() else {}
38+
39+
40+
class TestConfigSetCorruption:
41+
"""`gitt config set` must not destroy other keys when the file is corrupt."""
42+
43+
def test_corrupt_config_aborts_with_nonzero_exit(self, temp_config_dir):
44+
config_dir, config_file = temp_config_dir
45+
config_dir.mkdir(parents=True)
46+
# Truncated JSON — simulates an interrupted write or manual edit.
47+
config_file.write_text('{"network": "test"')
48+
49+
runner = CliRunner()
50+
result = runner.invoke(config_group, ['set', 'hotkey', 'default'])
51+
52+
assert result.exit_code != 0
53+
assert 'not valid JSON' in result.output
54+
assert 'Refusing to overwrite' in result.output
55+
56+
def test_corrupt_config_preserves_existing_file(self, temp_config_dir):
57+
"""The whole point: the bad file is left alone, not clobbered."""
58+
config_dir, config_file = temp_config_dir
59+
config_dir.mkdir(parents=True)
60+
original_bytes = b'{"network": "test"'
61+
config_file.write_bytes(original_bytes)
62+
63+
runner = CliRunner()
64+
runner.invoke(config_group, ['set', 'hotkey', 'default'])
65+
66+
# Byte-for-byte identical: the operator can recover the values they
67+
# had configured before the corruption.
68+
assert config_file.read_bytes() == original_bytes
69+
70+
def test_valid_config_still_round_trips(self, temp_config_dir):
71+
"""Non-corrupt files must still merge new keys without loss."""
72+
config_dir, config_file = temp_config_dir
73+
config_dir.mkdir(parents=True)
74+
config_file.write_text(json.dumps({'network': 'test', 'wallet': 'alice'}))
75+
76+
runner = CliRunner()
77+
result = runner.invoke(config_group, ['set', 'hotkey', 'default'])
78+
79+
assert result.exit_code == 0, result.output
80+
assert _read(config_file) == {'network': 'test', 'wallet': 'alice', 'hotkey': 'default'}
81+
82+
def test_missing_config_creates_fresh(self, temp_config_dir):
83+
"""First-run case: no existing file is fine — write a fresh one."""
84+
_, config_file = temp_config_dir
85+
assert not config_file.exists()
86+
87+
runner = CliRunner()
88+
result = runner.invoke(config_group, ['set', 'wallet', 'alice'])
89+
90+
assert result.exit_code == 0, result.output
91+
assert _read(config_file) == {'wallet': 'alice'}

0 commit comments

Comments
 (0)