From eab2074285bf65397b9fb22e7e557533d5e372a1 Mon Sep 17 00:00:00 2001 From: GrapeS Date: Wed, 19 Aug 2026 17:26:10 +0800 Subject: [PATCH 1/6] fix(render): redact a witness key that sits on its own line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RedactWitnessLine decides per line, and only the opening line of localwitness = [ ] starts with the key name. The element line — the one that actually carries the key — was returned unchanged, so it reached every surface that emits config lines: plan --diff, config diff, verify-config, and the MCP drift tool, whose output is handed to a third-party model provider. This is the shape the shipped private-network template uses, so a witness deployed from it leaked its key on any of those paths. Add RedactWitnessLines, which walks a whole config at once and stays inside an unterminated localwitness array, and move the four diff surfaces onto it. Comparison still runs over the raw lines, so a rotated key is still reported as a change. --- cmd/config/diff.go | 8 +++-- cmd/plan.go | 8 +++-- cmd/verify_config.go | 12 +++++--- internal/mcp/tools_drift.go | 15 ++++++---- internal/render/hocon.go | 55 +++++++++++++++++++++++++++++++++++ internal/render/hocon_test.go | 39 +++++++++++++++++++++++++ 6 files changed, 124 insertions(+), 13 deletions(-) diff --git a/cmd/config/diff.go b/cmd/config/diff.go index 58af5a0c..13a40b0d 100644 --- a/cmd/config/diff.go +++ b/cmd/config/diff.go @@ -122,6 +122,10 @@ func runDiff(cmd *cobra.Command, args []string) error { // assignment misaligns the tail and would otherwise print the SR // private key into `diffs[]`. func simpleDiff(old, new []string) []string { + // Redact whole-slice: a multi-line `localwitness = [` array keeps its + // key on a line that does not itself start with the key name. + oldR := render.RedactWitnessLines(old) + newR := render.RedactWitnessLines(new) var diffs []string maxLen := len(old) @@ -139,10 +143,10 @@ func simpleDiff(old, new []string) []string { } if oldLine != newLine { if oldLine != "" { - diffs = append(diffs, fmt.Sprintf("- %s", render.RedactWitnessLine(oldLine))) + diffs = append(diffs, fmt.Sprintf("- %s", oldR[i])) } if newLine != "" { - diffs = append(diffs, fmt.Sprintf("+ %s", render.RedactWitnessLine(newLine))) + diffs = append(diffs, fmt.Sprintf("+ %s", newR[i])) } } } diff --git a/cmd/plan.go b/cmd/plan.go index 9b963688..ceebf903 100644 --- a/cmd/plan.go +++ b/cmd/plan.go @@ -200,6 +200,10 @@ func runPlan(cmd *cobra.Command, args []string) error { // tail and would otherwise push the SR private key straight into // stdout and into result["config_diff"]. func simpleHOCONDiff(old, new []string) []string { + // Redact whole-slice: a multi-line `localwitness = [` array keeps its + // key on a line that does not itself start with the key name. + oldR := render.RedactWitnessLines(old) + newR := render.RedactWitnessLines(new) var diffs []string maxLen := len(old) if len(new) > maxLen { @@ -215,10 +219,10 @@ func simpleHOCONDiff(old, new []string) []string { } if oldLine != newLine { if oldLine != "" { - diffs = append(diffs, "- "+render.RedactWitnessLine(oldLine)) + diffs = append(diffs, "- "+oldR[i]) } if newLine != "" { - diffs = append(diffs, "+ "+render.RedactWitnessLine(newLine)) + diffs = append(diffs, "+ "+newR[i]) } } } diff --git a/cmd/verify_config.go b/cmd/verify_config.go index 4ff63582..101c741c 100644 --- a/cmd/verify_config.go +++ b/cmd/verify_config.go @@ -166,6 +166,10 @@ func readLiveConfig(ctx context.Context, nc *nodeContext, name string) (string, func lineDiff(live, desired string, contextLines int) []string { a := strings.Split(strings.TrimRight(live, "\n"), "\n") b := strings.Split(strings.TrimRight(desired, "\n"), "\n") + // Redact whole-slice: a multi-line `localwitness = [` array keeps its + // key on a line that does not itself start with the key name. + aR := render.RedactWitnessLines(a) + bR := render.RedactWitnessLines(b) var diffs []string max := len(a) if len(b) > max { @@ -192,17 +196,17 @@ func lineDiff(live, desired string, contextLines int) []string { } for j := lo; j < i; j++ { if j < len(a) { - diffs = append(diffs, " "+render.RedactWitnessLine(a[j])) + diffs = append(diffs, " "+aR[j]) } } } switch { case i < len(a) && i >= len(b): - diffs = append(diffs, "- "+render.RedactWitnessLine(aLine)) + diffs = append(diffs, "- "+aR[i]) case i >= len(a) && i < len(b): - diffs = append(diffs, "+ "+render.RedactWitnessLine(bLine)) + diffs = append(diffs, "+ "+bR[i]) default: - diffs = append(diffs, "- "+render.RedactWitnessLine(aLine), "+ "+render.RedactWitnessLine(bLine)) + diffs = append(diffs, "- "+aR[i], "+ "+bR[i]) } } return diffs diff --git a/internal/mcp/tools_drift.go b/internal/mcp/tools_drift.go index 3770a3fa..f1d43cdc 100644 --- a/internal/mcp/tools_drift.go +++ b/internal/mcp/tools_drift.go @@ -111,6 +111,11 @@ func mcpLineDiff(live, desired string, ctxLines int) []string { if len(b) > maxLen { maxLen = len(b) } + // Redact whole-slice: a multi-line `localwitness = [` array keeps its + // key on a line that does not itself start with the key name. This + // output leaves the machine, so the per-line pass is not enough. + aR := render.RedactWitnessLines(a) + bR := render.RedactWitnessLines(b) for i := range maxLen { var aLine, bLine string if i < len(a) { @@ -129,18 +134,18 @@ func mcpLineDiff(live, desired string, ctxLines int) []string { } for j := lo; j < i; j++ { if j < len(a) { - diffs = append(diffs, " "+render.RedactWitnessLine(a[j])) + diffs = append(diffs, " "+aR[j]) } } } switch { case i < len(a) && i >= len(b): - diffs = append(diffs, "- "+render.RedactWitnessLine(aLine)) + diffs = append(diffs, "- "+aR[i]) case i >= len(a) && i < len(b): - diffs = append(diffs, "+ "+render.RedactWitnessLine(bLine)) + diffs = append(diffs, "+ "+bR[i]) default: - diffs = append(diffs, "- "+render.RedactWitnessLine(aLine)) - diffs = append(diffs, "+ "+render.RedactWitnessLine(bLine)) + diffs = append(diffs, "- "+aR[i]) + diffs = append(diffs, "+ "+bR[i]) } } return diffs diff --git a/internal/render/hocon.go b/internal/render/hocon.go index 1562a5d5..303dd723 100644 --- a/internal/render/hocon.go +++ b/internal/render/hocon.go @@ -131,6 +131,61 @@ func RedactWitnessLine(line string) string { return lineIndent(line) + redactedWitnessAssignment } +// redactedWitnessElement stands in for a key that sits on its own line +// inside a multi-line `localwitness = [ ... ]` array. +const redactedWitnessElement = `""` + +// RedactWitnessLines redacts a whole config's worth of lines at once and +// returns a slice of the same length, so callers can keep comparing the +// raw lines positionally while emitting the redacted ones. +// +// It exists because RedactWitnessLine, looking at one line in isolation, +// cannot see the shape the shipped templates actually use: +// +// localwitness = [ +// +// ] +// +// Only the opening line begins with the `localwitness` key, so a per-line +// pass leaves the element line — the one that carries the key material — +// untouched. Every surface that emits config lines (plan --diff, config +// diff, verify-config and the MCP drift tool, whose output leaves the +// machine) must go through this rather than mapping RedactWitnessLine +// over the slice. +func RedactWitnessLines(lines []string) []string { + out := make([]string, len(lines)) + inArray := false + for i, line := range lines { + trimmed := strings.TrimSpace(line) + switch { + case inArray: + // The closing bracket carries no key material; anything + // before it does. + if strings.HasPrefix(trimmed, "]") { + inArray = false + out[i] = line + continue + } + if trimmed == "" || strings.HasPrefix(trimmed, "#") || + strings.HasPrefix(trimmed, "//") { + out[i] = line + continue + } + out[i] = lineIndent(line) + redactedWitnessElement + case IsWitnessKeyLine(line): + out[i] = lineIndent(line) + redactedWitnessAssignment + // An assignment that opens an array without closing it on + // the same line continues on the lines that follow. + if !strings.Contains(trimmed, "]") { + inArray = true + } + default: + out[i] = line + } + } + return out +} + // RenderHOCON loads the base template for the network and applies intent-driven overrides. // Returns the final HOCON config as a string. templateDir may be empty, in // which case the embedded template is used. diff --git a/internal/render/hocon_test.go b/internal/render/hocon_test.go index 4044e661..7c92aefb 100644 --- a/internal/render/hocon_test.go +++ b/internal/render/hocon_test.go @@ -536,3 +536,42 @@ func TestRenderHOCON_MonitoringDisabled(t *testing.T) { } } } + +// A localwitness array whose key sits on its own line must not survive +// redaction: only the opening line starts with the key name, so a +// per-line pass used to let the key itself straight through into +// `plan --diff`, `config diff`, `verify-config` and the MCP drift tool. +func TestRedactWitnessLinesMultiLineArray(t *testing.T) { + const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + in := []string{ + "storage = {", + "localwitness = [", + " " + key + " # you must enable this value", + "]", + "block = {", + } + got := RedactWitnessLines(in) + if len(got) != len(in) { + t.Fatalf("length changed: got %d want %d", len(got), len(in)) + } + for i, line := range got { + if strings.Contains(line, key) { + t.Errorf("line %d leaked the witness key: %q", i, line) + } + } + if got[0] != in[0] || got[4] != in[4] { + t.Errorf("unrelated lines were rewritten: %q %q", got[0], got[4]) + } + if got[3] != "]" { + t.Errorf("closing bracket rewritten: %q", got[3]) + } +} + +// The single-line form keeps behaving exactly as before. +func TestRedactWitnessLinesSingleLine(t *testing.T) { + in := []string{`localwitness = ["deadbeef"]`} + got := RedactWitnessLines(in) + if strings.Contains(got[0], "deadbeef") { + t.Fatalf("single-line key leaked: %q", got[0]) + } +} From 6ea36d56138e87765b17ccf939aeaebb42d6bea0 Mon Sep 17 00:00:00 2001 From: GrapeS Date: Wed, 19 Aug 2026 17:33:53 +0800 Subject: [PATCH 2/6] chore(private-net): rotate the demo witness key to an empty account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The demo key shipped in the private-network template derives TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY, which holds funds on the public chains — 26,568 TRX on Nile and 1 TRX on mainnet at the time of writing. A key published in a repository must not control anything, and anyone following the template was pointed at an account that does. Replace it with a freshly generated pair whose address does not exist on either chain. The template uses the address in three places that have to agree or the private chain will not produce blocks — the genesis allocation, the genesis witness list, and the localwitness key — so all three move together. old TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY new TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi The pair was generated with tools/txgen and checked by deriving the address back from the private key. --- AGENTS.md | 2 +- examples/token-lab/README.md | 2 +- internal/knowledge/files/test-harness.md | 6 +++--- internal/render/hocon_test.go | 2 +- internal/render/templates/private_net_config.conf | 6 +++--- internal/render/witness_redaction_test.go | 4 ++-- knowledge/test-harness.md | 6 +++--- private_net_config.conf | 6 +++--- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5de91a2f..0dd1a519 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -394,7 +394,7 @@ trond preflight --intent my-net.yaml -o json # 4. Create the whole network in one shot. trond auto-wires # node.active between siblings so peering works under auto_ports. -SR_KEY=da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 \ +SR_KEY=a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae \ trond network create --intent my-net.yaml --wait -o json # Output: {"network":"pn", "nodes":[{"name":"pn-node0", "endpoints":{...}}, ...]} diff --git a/examples/token-lab/README.md b/examples/token-lab/README.md index bb72b57b..8352b441 100644 --- a/examples/token-lab/README.md +++ b/examples/token-lab/README.md @@ -5,7 +5,7 @@ through it with `txgen`, and assert the receivers hold exactly what was sent. ```bash npm install # once — tronweb, for signing -export SR_PRIVATE_KEY=da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 +export SR_PRIVATE_KEY=a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae trond recipe run --file examples/token-lab/trc20.yaml --allow-host-exec \ --param lab_dir=examples/token-lab --param sender_key=$SR_PRIVATE_KEY diff --git a/internal/knowledge/files/test-harness.md b/internal/knowledge/files/test-harness.md index 970210fb..b67c5f79 100644 --- a/internal/knowledge/files/test-harness.md +++ b/internal/knowledge/files/test-harness.md @@ -176,7 +176,7 @@ Driver script: ```bash docker network create tron-pn-mesh -SR_KEY=da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 \ +SR_KEY=a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae \ trond --state-dir /tmp/trond-$JOB \ network create --intent pn.yaml -o json ``` @@ -191,8 +191,8 @@ docker run --rm -v pn-node1_pn-node1-logs:/L alpine \ ``` The default private-net witness private key -(`da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0`) matches -the genesis address `TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY` baked into the +(`a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae`) matches +the genesis address `TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi` baked into the `private_net_config.conf` template. Use that exact key unless you also supply a fresh genesis block. diff --git a/internal/render/hocon_test.go b/internal/render/hocon_test.go index 7c92aefb..d295c8ce 100644 --- a/internal/render/hocon_test.go +++ b/internal/render/hocon_test.go @@ -542,7 +542,7 @@ func TestRenderHOCON_MonitoringDisabled(t *testing.T) { // per-line pass used to let the key itself straight through into // `plan --diff`, `config diff`, `verify-config` and the MCP drift tool. func TestRedactWitnessLinesMultiLineArray(t *testing.T) { - const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + const key = "a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae" in := []string{ "storage = {", "localwitness = [", diff --git a/internal/render/templates/private_net_config.conf b/internal/render/templates/private_net_config.conf index 52e79a85..8c58f192 100644 --- a/internal/render/templates/private_net_config.conf +++ b/internal/render/templates/private_net_config.conf @@ -449,7 +449,7 @@ genesis.block = { { accountName = "Zion" accountType = "AssetIssue" - address = "TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY" + address = "TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi" balance = "95000000000000000" }, { @@ -498,7 +498,7 @@ genesis.block = { witnesses = [ { - address: TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY, + address: TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi, url = "http://tronstudio.com", voteCount = 10000 } @@ -518,7 +518,7 @@ genesis.block = { //localWitnessAccountAddress = localwitness = [ - da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 # you must enable this value and the witness address TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY are matched. + a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae # you must enable this value and the witness address TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi are matched. ] #localwitnesskeystore = [ diff --git a/internal/render/witness_redaction_test.go b/internal/render/witness_redaction_test.go index 3988aecb..064bb980 100644 --- a/internal/render/witness_redaction_test.go +++ b/internal/render/witness_redaction_test.go @@ -138,7 +138,7 @@ func TestRendered_NoSecret_FormsAreIdentical(t *testing.T) { Type: "witness", WitnessKey: &intent.WitnessKey{ KeystorePath: "/opt/tron/keystore.json", - AccountAddress: "TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY", + AccountAddress: "TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi", }, }, "witness-no-key": {Type: "witness"}, @@ -229,7 +229,7 @@ func TestIsWitnessKeyLine_ExactKeyMatch(t *testing.T) { noMatch := []string{ `localwitnesskeystore = ["/opt/tron/keystore.json"]`, `#localwitnesskeystore = [`, - `localWitnessAccountAddress = "TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY"`, + `localWitnessAccountAddress = "TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi"`, `# and the localwitness is configured with the private key`, `// When it is empty,the localwitness is configured with the private key`, `# localWitnessAccountAddress =`, diff --git a/knowledge/test-harness.md b/knowledge/test-harness.md index 970210fb..b67c5f79 100644 --- a/knowledge/test-harness.md +++ b/knowledge/test-harness.md @@ -176,7 +176,7 @@ Driver script: ```bash docker network create tron-pn-mesh -SR_KEY=da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 \ +SR_KEY=a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae \ trond --state-dir /tmp/trond-$JOB \ network create --intent pn.yaml -o json ``` @@ -191,8 +191,8 @@ docker run --rm -v pn-node1_pn-node1-logs:/L alpine \ ``` The default private-net witness private key -(`da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0`) matches -the genesis address `TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY` baked into the +(`a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae`) matches +the genesis address `TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi` baked into the `private_net_config.conf` template. Use that exact key unless you also supply a fresh genesis block. diff --git a/private_net_config.conf b/private_net_config.conf index 52e79a85..8c58f192 100644 --- a/private_net_config.conf +++ b/private_net_config.conf @@ -449,7 +449,7 @@ genesis.block = { { accountName = "Zion" accountType = "AssetIssue" - address = "TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY" + address = "TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi" balance = "95000000000000000" }, { @@ -498,7 +498,7 @@ genesis.block = { witnesses = [ { - address: TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY, + address: TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi, url = "http://tronstudio.com", voteCount = 10000 } @@ -518,7 +518,7 @@ genesis.block = { //localWitnessAccountAddress = localwitness = [ - da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0 # you must enable this value and the witness address TPL66VK2gCXNCD7EJg9pgJRfqcRazjhUZY are matched. + a31d54825aea2fc5127e3bd435fc2346021313005e5f304ab33372432784acae # you must enable this value and the witness address TM4yToQ1njkcFwi3ADY5x6dbdfNekU3rVi are matched. ] #localwitnesskeystore = [ From 92570fc12e2d6cc82d37acf45a2767f273a36bed Mon Sep 17 00:00:00 2001 From: GrapeS Date: Wed, 19 Aug 2026 17:33:53 +0800 Subject: [PATCH 3/6] build: move to Go 1.25.13 and x/net v0.55.0 to clear govulncheck `make vuln` reported five advisories reachable from this code: GO-2026-6218 net/url quadratic resolvePath GO-2026-6090 crypto/tls GO-2026-5972 encoding/asn1 recursion depth GO-2026-5856 crypto/tls ECH privacy leak GO-2026-5026 x/net/idna ASCII-only punycode labels The stdlib ones need go1.25.13; the last also needs x/net v0.55.0. The Makefile pinned 1.25.9 while go.mod asked for 1.25.11, so the bootstrap and the toolchain directive named different versions; both now say 1.25.13. The four tarball hashes in bootstrap-go.sh come from go.dev/dl/?mode=json, as the comment there requires. `make vuln` now reports no vulnerabilities. --- Makefile | 2 +- go.mod | 4 ++-- go.sum | 2 ++ scripts/bootstrap-go.sh | 10 +++++----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 9db5f657..0e06b66a 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ LDFLAGS := -s -w -X $(MODULE)/cmd.version=$(VERSION) -X $(MODULE)/cmd.commit= # resolves on PATH (useful in CI runners that already pinned Go via # actions/setup-go and want to skip the download step). -GO_VERSION ?= 1.25.9 +GO_VERSION ?= 1.25.13 ifeq ($(USE_SYSTEM_GO),1) GO := go diff --git a/go.mod b/go.mod index f10b62d9..928f5e1c 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/tronprotocol/tron-deployment go 1.25.0 -toolchain go1.25.11 +toolchain go1.25.13 require ( github.com/cloudflare/circl v1.6.3 @@ -51,7 +51,7 @@ require ( go.opentelemetry.io/otel/metric v1.43.0 // indirect go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/net v0.54.0 // indirect + golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/text v0.37.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect diff --git a/go.sum b/go.sum index 10777cb2..a696f3d6 100644 --- a/go.sum +++ b/go.sum @@ -116,6 +116,8 @@ golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGb golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/scripts/bootstrap-go.sh b/scripts/bootstrap-go.sh index d85e5750..902462fc 100755 --- a/scripts/bootstrap-go.sh +++ b/scripts/bootstrap-go.sh @@ -21,7 +21,7 @@ SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) PROJECT_ROOT=$(cd -- "$SCRIPT_DIR/.." && pwd) cd "$PROJECT_ROOT" -GO_VERSION=${GO_VERSION:-1.25.9} +GO_VERSION=${GO_VERSION:-1.25.13} GO_DIR=".go-toolchain/${GO_VERSION}" # Per-platform tarball + sha256. When bumping GO_VERSION, refresh the @@ -52,10 +52,10 @@ url="https://go.dev/dl/${archive}" # Each line is " ". Pulled from go.dev/dl/?mode=json # at the time GO_VERSION was set. case "${os}-${arch}" in - linux-amd64) expected_sha="00859d7bd6defe8bf84d9db9e57b9a4467b2887c18cd93ae7460e713db774bc1" ;; - linux-arm64) expected_sha="ec342e7389b7f489564ed5463c63b16cf8040023dabc7861256677165a8c0e2b" ;; - darwin-amd64) expected_sha="92cb78fba4796e218c1accb0ea0a214ef2094c382049a244ad6505505d015fbe" ;; - darwin-arm64) expected_sha="9528be7329b9770631a6bd09ca2f3a73ed7332bec01d87435e75e92d8f130363" ;; + linux-amd64) expected_sha="39042a078ea9ceebe3ecda4a7188f0f5b96e14a071d27923ba7f40b456e85ae3" ;; + linux-arm64) expected_sha="adad240fcb6bd180cf973b4b7c747baf4ec81d08b7d40ca35940ee4531971490" ;; + darwin-amd64) expected_sha="d742b7a53f8c8be5e02d75263883482cebabbe14ec9308cb056dd8aebeb040df" ;; + darwin-arm64) expected_sha="916fe61a2bc78dd516b3629ee3428b06e17141b85a70f1986c260149a3d2ffbd" ;; *) echo "bootstrap-go: no SHA256 recorded for ${os}-${arch}" >&2 exit 1 From 9d65e098cdeb94554409c10f421643bf6b65f358 Mon Sep 17 00:00:00 2001 From: GrapeS Date: Thu, 20 Aug 2026 10:43:39 +0800 Subject: [PATCH 4/6] fix(bootstrap): let host preparation run over SSH again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSH whitelist deliberately dropped apt-get, yum and the shell — a wide list would hand the SSH user's authority to anyone who can run `trond exec`, which passes whichever command name the caller gives. The comment on allowedCommands says package managers "can be re-allowed scoped to that one command path if needed", but bootstrap kept calling them, so `trond bootstrap` failed on its first apt-get against any SSH target. It only ever worked locally, where LocalTarget.Exec does not consult the whitelist, and installDocker/installJDK have no test. Add that scoping: a provisioning set (apt-get, yum, useradd, sh) that only applies to a target SSHTarget.SetProvisioning has switched on, and only bootstrap calls it. SetProvisioning is not part of target.Target, so no lifecycle path and no `trond exec` can reach it. Anything outside both sets — curl and wget among them — is still refused. --- cmd/bootstrap.go | 7 ++++++ internal/security/ssh_whitelist.go | 26 +++++++++++++++++++++ internal/security/ssh_whitelist_test.go | 30 +++++++++++++++++++++++++ internal/target/ssh.go | 16 ++++++++++++- 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 internal/security/ssh_whitelist_test.go diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index bdda241b..253b54cb 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -41,6 +41,13 @@ func runBootstrap(cmd *cobra.Command, args []string) error { if closer, ok := tgt.(interface{ Close() error }); ok { defer closer.Close() } + // Host preparation installs packages, which the ordinary SSH whitelist + // does not allow — deliberately, so that no lifecycle path or `trond + // exec` can. bootstrap is the one command that may, and only for the + // lifetime of this target. + if p, ok := tgt.(interface{ SetProvisioning(bool) }); ok { + p.SetProvisioning(true) + } runtimeType := parsed.Target.Runtime if runtimeType == "" { diff --git a/internal/security/ssh_whitelist.go b/internal/security/ssh_whitelist.go index a1086e7a..155dc0db 100644 --- a/internal/security/ssh_whitelist.go +++ b/internal/security/ssh_whitelist.go @@ -61,6 +61,32 @@ func ValidateCommand(cmd string) error { return nil } +// provisioningCommands are allowed on top of allowedCommands, and only for a +// target that has been put in provisioning mode. `trond bootstrap` is the one +// caller that does so, which is the scoping the comment on allowedCommands +// asks for: these names install packages and run the vendor's Docker +// installer, so they must not be reachable from `trond exec` or from any +// lifecycle path driven by intent fields. +var provisioningCommands = map[string]bool{ + "apt-get": true, + "yum": true, + "useradd": true, + // The Docker convenience script is delivered by a pipeline, so bootstrap + // hands it to a shell. Nothing outside bootstrap may do this. + "sh": true, +} + +// ValidateProvisioningCommand accepts the ordinary whitelist plus the +// package-manager and shell commands bootstrap needs. +func ValidateProvisioningCommand(cmd string) error { + base := extractBaseCommand(cmd) + if allowedCommands[base] || provisioningCommands[base] { + return nil + } + return fmt.Errorf("command %q is allowed neither by the SSH whitelist nor in provisioning mode; allowed commands: %s", + base, allowedCommandList()) +} + // extractBaseCommand gets the first word (the command name) from a command string. func extractBaseCommand(cmd string) string { // Handle sudo prefix diff --git a/internal/security/ssh_whitelist_test.go b/internal/security/ssh_whitelist_test.go new file mode 100644 index 00000000..f62396d2 --- /dev/null +++ b/internal/security/ssh_whitelist_test.go @@ -0,0 +1,30 @@ +package security + +import "testing" + +// Package managers and the shell must stay out of the ordinary whitelist: +// `trond exec` passes whichever name the caller gives, so allowing them +// globally would hand the SSH user's authority to anyone who can run it. +func TestProvisioningCommandsAreNotGloballyAllowed(t *testing.T) { + for _, cmd := range []string{"apt-get", "yum", "useradd", "sh"} { + if err := ValidateCommand(cmd); err == nil { + t.Errorf("%q must not be in the ordinary whitelist", cmd) + } + } +} + +// bootstrap needs them, so provisioning mode accepts them — and still +// accepts everything the ordinary whitelist does. +func TestValidateProvisioningCommand(t *testing.T) { + for _, cmd := range []string{"apt-get", "yum", "useradd", "sh", "docker", "which"} { + if err := ValidateProvisioningCommand(cmd); err != nil { + t.Errorf("provisioning should allow %q: %v", cmd, err) + } + } + // Widening is bounded: anything outside both sets is still refused. + for _, cmd := range []string{"curl", "wget", "nc", "python3"} { + if err := ValidateProvisioningCommand(cmd); err == nil { + t.Errorf("provisioning must still refuse %q", cmd) + } + } +} diff --git a/internal/target/ssh.go b/internal/target/ssh.go index fe2f68be..69d23203 100644 --- a/internal/target/ssh.go +++ b/internal/target/ssh.go @@ -33,6 +33,10 @@ type SSHTarget struct { identityFile string knownHostsFile string // Path to known_hosts; empty uses ~/.ssh/known_hosts client *ssh.Client + // provisioning widens the command whitelist to the package managers and + // shell that host preparation needs. Only SetProvisioning sets it, and + // only `trond bootstrap` calls that. + provisioning bool } func (t *SSHTarget) DialContext(_ context.Context, network, addr string) (net.Conn, error) { @@ -52,6 +56,12 @@ func (t *SSHTarget) DialContext(_ context.Context, network, addr string) (net.Co return t.client.Dial(network, addr) } +// SetProvisioning puts the target in provisioning mode, which additionally +// allows the package-manager and shell commands host preparation needs. It is +// deliberately not part of the target.Target interface: no lifecycle code path +// and no `trond exec` invocation can reach it. +func (t *SSHTarget) SetProvisioning(on bool) { t.provisioning = on } + // NewSSHTarget creates a new SSHTarget. Call Connect() before use. func NewSSHTarget(host string, port int, user, identityFile string) *SSHTarget { if port == 0 { @@ -200,7 +210,11 @@ func (t *SSHTarget) Exec(ctx context.Context, cmd string, args ...string) ([]byt return nil, fmt.Errorf("ssh not connected") } - if err := security.ValidateCommand(cmd); err != nil { + validate := security.ValidateCommand + if t.provisioning { + validate = security.ValidateProvisioningCommand + } + if err := validate(cmd); err != nil { return nil, err } From d8bc344fda12fdfd145cef8b2f84fa152f9b7b38 Mon Sep 17 00:00:00 2001 From: GrapeS Date: Thu, 20 Aug 2026 10:47:22 +0800 Subject: [PATCH 5/6] fix(state): hold the lock across every load-modify-save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only apply and the MCP lifecycle tool took the state lock. Every other writer — start, stop, restart, upgrade, rollback, heal, remove and the three network commands — read the node list, worked for as long as the operation took, then wrote the whole list back, with no lock in between. Two trond processes therefore both start from the same list and the second save drops whatever the first added. A test that runs the unlocked cycle twice concurrently loses an update 200 times out of 200, so this is what happens rather than a narrow race. Take the lock where the read happens: - nodeContext acquires it in resolveNodeContext and releases it in Close, which covers the eight commands built on it. apply does not use nodeContext and keeps its own lock, and `network upgrade` only spawns children, so nothing nests an acquire inside another. - the three network commands and the MCP heal tool take it directly, matching what the lifecycle tool already did. Save also wrote through a fixed ".tmp". Concurrent writers share that name, overwrite each other's half-written file and then rename whatever is there, which defeats the point of writing to a temp file at all. Use a unique name per call and chmod it to 0600, since CreateTemp makes the file 0600 already but that is not part of its contract. --- cmd/network/add.go | 9 +++ cmd/network/create.go | 9 +++ cmd/network/destroy.go | 9 +++ cmd/resolve.go | 29 ++++++++- internal/mcp/tools_heal.go | 9 +++ internal/state/lock_concurrency_test.go | 85 +++++++++++++++++++++++++ internal/state/store.go | 23 ++++++- 7 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 internal/state/lock_concurrency_test.go diff --git a/cmd/network/add.go b/cmd/network/add.go index 4c9a43f0..58f2c664 100644 --- a/cmd/network/add.go +++ b/cmd/network/add.go @@ -73,6 +73,15 @@ func runAdd(cmd *cobra.Command, args []string) error { // Pick the next free index. Existing entries are "-node"; we // rescan state instead of trusting any in-memory counter so the operation // is safe to retry. + // Hold the state lock across the whole load-modify-save cycle: this + // command reads the node list here and writes it back much later, and + // a concurrent trond would otherwise drop one of the two updates. + lock := state.NewLock(paths.BaseDir()) + if err := lock.Acquire(); err != nil { + return output.NewError("LOCK_ERROR", output.ExitGeneralError, "acquire state lock: "+err.Error()) + } + defer lock.Release() + store, err := state.NewStore(paths.State()) if err != nil { return err diff --git a/cmd/network/create.go b/cmd/network/create.go index 483b72bf..9f930ef4 100644 --- a/cmd/network/create.go +++ b/cmd/network/create.go @@ -85,6 +85,15 @@ func runCreate(cmd *cobra.Command, args []string) error { templateDir := findTemplatesDir() workDir := paths.Deployments() + // Hold the state lock across the whole load-modify-save cycle: this + // command reads the node list here and writes it back much later, and + // a concurrent trond would otherwise drop one of the two updates. + lock := state.NewLock(paths.BaseDir()) + if err := lock.Acquire(); err != nil { + return output.NewError("LOCK_ERROR", output.ExitGeneralError, "acquire state lock: "+err.Error()) + } + defer lock.Release() + store, err := state.NewStore(paths.State()) if err != nil { return output.NewError("STATE_ERROR", output.ExitGeneralError, err.Error()) diff --git a/cmd/network/destroy.go b/cmd/network/destroy.go index 4d6719f8..6722823f 100644 --- a/cmd/network/destroy.go +++ b/cmd/network/destroy.go @@ -39,6 +39,15 @@ func runDestroy(cmd *cobra.Command, args []string) error { WithSuggestions("Add --confirm to proceed") } + // Hold the state lock across the whole load-modify-save cycle: this + // command reads the node list here and writes it back much later, and + // a concurrent trond would otherwise drop one of the two updates. + lock := state.NewLock(paths.BaseDir()) + if err := lock.Acquire(); err != nil { + return output.NewError("LOCK_ERROR", output.ExitGeneralError, "acquire state lock: "+err.Error()) + } + defer lock.Release() + store, err := state.NewStore(paths.State()) if err != nil { return err diff --git a/cmd/resolve.go b/cmd/resolve.go index c8b9668e..573280c5 100644 --- a/cmd/resolve.go +++ b/cmd/resolve.go @@ -22,13 +22,24 @@ type nodeContext struct { Node *state.ManagedNode Target target.Target Runtime runtime.Runtime + + // lock is held for the whole load-modify-save cycle. Every command + // built on a nodeContext reads state here and writes it back through + // SaveState, sometimes many seconds later, so the read and the write + // have to sit inside one lock or a concurrent trond drops one of the + // two updates. + lock *state.Lock } -// Close releases resources (e.g., SSH connections). +// Close releases the state lock and any resources (e.g., SSH connections). func (nc *nodeContext) Close() { if closer, ok := nc.Target.(interface{ Close() error }); ok { closer.Close() } + if nc.lock != nil { + nc.lock.Release() + nc.lock = nil + } } // SaveState persists the current deployment state. @@ -111,13 +122,27 @@ func resolveNodeContext(name string) (*nodeContext, error) { return nil, err } + // Take the lock before the read: the caller writes the same state back + // through SaveState once its operation finishes. + lock := state.NewLock(stateDir()) + if err := lock.Acquire(); err != nil { + return nil, exitWithError("LOCK_ERROR", output.ExitGeneralError, + "Failed to acquire state lock: "+err.Error(), + "Check if another trond process is running") + } + release := func() { + lock.Release() + } + deployState, err := store.Load() if err != nil { + release() return nil, err } node := store.GetNode(deployState, name) if node == nil { + release() return nil, exitWithError("NODE_NOT_FOUND", output.ExitGeneralError, fmt.Sprintf("Node %q not found in state", name), "Run: trond list", @@ -126,6 +151,7 @@ func resolveNodeContext(name string) (*nodeContext, error) { tgt, err := resolveTargetFromNode(node) if err != nil { + release() return nil, exitWithError("TARGET_UNREACHABLE", output.ExitTargetUnreachable, err.Error()) } @@ -137,6 +163,7 @@ func resolveNodeContext(name string) (*nodeContext, error) { Node: node, Target: tgt, Runtime: rt, + lock: lock, }, nil } diff --git a/internal/mcp/tools_heal.go b/internal/mcp/tools_heal.go index deec14b0..1b771e41 100644 --- a/internal/mcp/tools_heal.go +++ b/internal/mcp/tools_heal.go @@ -42,6 +42,15 @@ func autoHealTool(ctx context.Context, _ *mcp.CallToolRequest, args autoHealArgs return errResult(fmt.Errorf("name is required")) } + // Hold the state lock across the load-modify-save cycle, the same way + // the lifecycle tool does — heal writes the node list back after the + // repair runs. + lock := state.NewLock(paths.BaseDir()) + if err := lock.Acquire(); err != nil { + return errResult(fmt.Errorf("acquire state lock: %w", err)) + } + defer lock.Release() + store, err := state.NewStore(paths.State()) if err != nil { return errResult(err) diff --git a/internal/state/lock_concurrency_test.go b/internal/state/lock_concurrency_test.go new file mode 100644 index 00000000..393583c8 --- /dev/null +++ b/internal/state/lock_concurrency_test.go @@ -0,0 +1,85 @@ +package state + +import ( + "os" + "path/filepath" + "sync" + "testing" +) + +// The lock has to serialise a whole load-modify-save cycle. Without it the +// two goroutines below both read the same list, each append their own node, +// and whichever saves last wins — the other node is gone. +func TestLockSerialisesLoadModifySave(t *testing.T) { + dir := t.TempDir() + store, err := NewStore(filepath.Join(dir, "state.json")) + if err != nil { + t.Fatalf("new store: %v", err) + } + + var wg sync.WaitGroup + for _, name := range []string{"node-a", "node-b"} { + wg.Add(1) + go func(n string) { + defer wg.Done() + lock := NewLock(dir) + if err := lock.Acquire(); err != nil { + t.Errorf("acquire: %v", err) + return + } + defer lock.Release() + + st, err := store.Load() + if err != nil { + t.Errorf("load: %v", err) + return + } + store.UpsertNode(st, ManagedNode{Name: n}) + if err := store.Save(st); err != nil { + t.Errorf("save: %v", err) + } + }(name) + } + wg.Wait() + + st, err := store.Load() + if err != nil { + t.Fatalf("final load: %v", err) + } + if len(st.Nodes) != 2 { + t.Fatalf("lost an update: want 2 nodes, got %d (%+v)", len(st.Nodes), st.Nodes) + } +} + +// Save must not leave temp files behind, and must not collide when two +// writers run at once — a fixed ".tmp" name would have them overwrite each +// other's partially written file. +func TestSaveUsesUniqueTempFiles(t *testing.T) { + dir := t.TempDir() + store, err := NewStore(filepath.Join(dir, "state.json")) + if err != nil { + t.Fatalf("new store: %v", err) + } + var wg sync.WaitGroup + for i := range 8 { + wg.Add(1) + go func(i int) { + defer wg.Done() + st := &DeploymentState{Nodes: []ManagedNode{{Name: "n"}}} + if err := store.Save(st); err != nil { + t.Errorf("save %d: %v", i, err) + } + }(i) + } + wg.Wait() + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("readdir: %v", err) + } + for _, e := range entries { + if e.Name() != "state.json" { + t.Errorf("leftover file after Save: %s", e.Name()) + } + } +} diff --git a/internal/state/store.go b/internal/state/store.go index d38ec9c9..2a348c38 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -69,11 +69,28 @@ func (s *Store) Save(state *DeploymentState) error { return fmt.Errorf("marshal state: %w", err) } - // Atomic write: write to temp file then rename - tmpPath := s.path + ".tmp" - if err := os.WriteFile(tmpPath, data, 0600); err != nil { + // Atomic write: write to a temp file then rename. The temp name is + // unique per call — a fixed ".tmp" is shared by concurrent writers, + // which then overwrite each other's half-written file and rename + // whatever is left, so the rename stops being atomic in practice. + tmp, err := os.CreateTemp(filepath.Dir(s.path), filepath.Base(s.path)+".tmp*") + if err != nil { + return fmt.Errorf("create state temp file: %w", err) + } + tmpPath := tmp.Name() + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(tmpPath) return fmt.Errorf("write state temp file: %w", err) } + if err := tmp.Close(); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("close state temp file: %w", err) + } + if err := os.Chmod(tmpPath, 0600); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("chmod state temp file: %w", err) + } if err := os.Rename(tmpPath, s.path); err != nil { os.Remove(tmpPath) From bd240590b367d3837c84d89681335ced5f08940a Mon Sep 17 00:00:00 2001 From: GrapeS Date: Thu, 20 Aug 2026 14:51:39 +0800 Subject: [PATCH 6/6] fix: address review on #221 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things the review caught. The MCP conf resource handed out the live config with no redaction. It is the one surface whose full text leaves the machine by design — the client, and from there a model provider — and a witness node's config carries its signing key. Redact at the resource, not in the shared helper: the drift tool reads the same helper and needs the raw text, since a redacted side would report every witness node as drifted. RedactWitnessLines decided array membership from line shapes, and two shapes defeated it. A `]` inside the comment on the opening line ended the array before it began and let the key through; an element that closed the array left the scanner inside it, so every following line came out as and the diff said nothing. Find the values by parsing instead — the config parser is already a dependency for dbfork — and replace those literal strings wherever they appear, which no formatting can dodge. The scan stays as the fallback for text that does not parse, hardened to strip comments before the bracket checks. Every nodeContext command was holding the exclusive state lock until Close, including ones that never write: logs, wait, exec, files, health, diagnose, verify-config. A `trond logs -f` left running would block every other trond process on the host, and Acquire had no deadline, so the wait was indefinite and unexplained. Split the resolve path — readers take nothing, writers keep the lock across load-modify-save — and bound the wait at 30s with an error that says which process to look for. Enabling provisioning mode made a discarded error live: ensureServiceUser ignored useradd's result, which was invisible while the whitelist refused the call. Check it, tolerating only "already exists" so bootstrap stays re-runnable. --- cmd/bootstrap.go | 26 ++++++- cmd/bootstrap_test.go | 31 +++++++++ cmd/heal.go | 2 +- cmd/remove.go | 2 +- cmd/resolve.go | 71 ++++++++++++++++--- cmd/restart.go | 2 +- cmd/rollback.go | 2 +- cmd/start.go | 2 +- cmd/stop.go | 2 +- cmd/upgrade.go | 2 +- internal/mcp/conf_helpers.go | 15 ++++ internal/mcp/conf_redaction_test.go | 40 +++++++++++ internal/mcp/resources.go | 9 ++- internal/render/hocon.go | 103 ++++++++++++++++++++++++++-- internal/render/hocon_test.go | 74 ++++++++++++++++++++ 15 files changed, 359 insertions(+), 24 deletions(-) create mode 100644 cmd/bootstrap_test.go create mode 100644 internal/mcp/conf_redaction_test.go diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 253b54cb..a10aa121 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -77,7 +77,18 @@ func runBootstrap(cmd *cobra.Command, args []string) error { if len(parsed.Nodes) > 0 && parsed.Nodes[0].SystemUser != "" { user = parsed.Nodes[0].SystemUser } - tgt.Exec(ctx, "useradd", "--system", "--no-create-home", "--shell", "/usr/sbin/nologin", user) + // Until provisioning mode existed this call was refused by the + // SSH whitelist and failed on every remote target, so discarding + // the error was invisible. Now that it runs, a real failure — + // no permission, a conflicting uid, no /usr/sbin/nologin — would + // otherwise be reported as a created user. + if out, err := tgt.Exec(ctx, "useradd", "--system", "--no-create-home", + "--shell", "/usr/sbin/nologin", user); err != nil { + if !userAlreadyExists(out) { + return exitWithError("BOOTSTRAP_ERROR", output.ExitGeneralError, + fmt.Sprintf("Failed to create system user %q: %v: %s", user, err, strings.TrimSpace(string(out)))) + } + } installed = append(installed, "user:"+user) } @@ -138,3 +149,16 @@ func installJDK(ctx context.Context, tgt target.Target) error { return fmt.Errorf("unsupported package manager; install JDK 17 manually") } + +// userAlreadyExists reports whether a useradd failure was only the user +// being there already, which bootstrap has to tolerate: it is expected +// to be re-runnable, and the second run finds the user from the first. +// +// useradd exits 9 for "name already in use", but the exit status does +// not survive target.Exec's error, so the message is what is left to +// match on. Both util-linux and busybox wording are covered. +func userAlreadyExists(out []byte) bool { + msg := strings.ToLower(string(out)) + return strings.Contains(msg, "already exists") || + strings.Contains(msg, "already in use") +} diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go new file mode 100644 index 00000000..9d536023 --- /dev/null +++ b/cmd/bootstrap_test.go @@ -0,0 +1,31 @@ +package cmd + +import "testing" + +// bootstrap has to be re-runnable, so a useradd that fails only because +// the user is already there must not abort the run — but anything else +// must, now that provisioning mode lets the call actually execute. +func TestUserAlreadyExists(t *testing.T) { + tolerated := []string{ + "useradd: user 'tron' already exists", + "useradd: UID 999 is not unique\nuseradd: name tron already in use", + "ALREADY EXISTS", + } + for _, out := range tolerated { + if !userAlreadyExists([]byte(out)) { + t.Errorf("should tolerate: %q", out) + } + } + + fatal := []string{ + "useradd: Permission denied.", + "useradd: cannot open /etc/passwd", + "useradd: invalid shell '/usr/sbin/nologin'", + "", + } + for _, out := range fatal { + if userAlreadyExists([]byte(out)) { + t.Errorf("should not tolerate: %q", out) + } + } +} diff --git a/cmd/heal.go b/cmd/heal.go index d67dc79d..58dfaba1 100644 --- a/cmd/heal.go +++ b/cmd/heal.go @@ -102,7 +102,7 @@ func runAutoHeal(cmd *cobra.Command, args []string) error { } } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/remove.go b/cmd/remove.go index 2d443cf7..f0d65e1b 100644 --- a/cmd/remove.go +++ b/cmd/remove.go @@ -49,7 +49,7 @@ func runRemove(cmd *cobra.Command, args []string) error { } start := time.Now() - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/resolve.go b/cmd/resolve.go index 573280c5..5f227d3f 100644 --- a/cmd/resolve.go +++ b/cmd/resolve.go @@ -115,23 +115,43 @@ func requirePrivateForNodes(names ...string) error { return guard.EnforceNodes(refs) } -// resolveNodeContext loads a node from state and constructs its target and runtime. +// resolveNodeContext loads a node from state without keeping the state +// lock. Use it for commands that only read — logs, wait, exec, files, +// health, diagnose, verify-config. Holding the exclusive lock across +// those buys nothing, and `logs -f` or a long `wait` would keep every +// other trond process on the host blocked for as long as it runs. func resolveNodeContext(name string) (*nodeContext, error) { + return resolveNode(name, false) +} + +// resolveNodeContextForWrite loads a node and keeps the state lock until +// Close. Commands that write the node list back — start, stop, restart, +// upgrade, rollback, heal, remove — need the read and the write inside +// one lock, or a concurrent trond drops one of the two updates. +func resolveNodeContextForWrite(name string) (*nodeContext, error) { + return resolveNode(name, true) +} + +func resolveNode(name string, forWrite bool) (*nodeContext, error) { store, err := state.NewStore(statePath()) if err != nil { return nil, err } - // Take the lock before the read: the caller writes the same state back - // through SaveState once its operation finishes. - lock := state.NewLock(stateDir()) - if err := lock.Acquire(); err != nil { - return nil, exitWithError("LOCK_ERROR", output.ExitGeneralError, - "Failed to acquire state lock: "+err.Error(), - "Check if another trond process is running") + // A writer takes the lock before the read and holds it until Close, + // so the load-modify-save cycle is atomic. A reader takes nothing: + // the load below is a single Load() and nothing is written back. + var lock *state.Lock + if forWrite { + lock = state.NewLock(stateDir()) + if err := acquireStateLock(lock); err != nil { + return nil, err + } } release := func() { - lock.Release() + if lock != nil { + lock.Release() + } } deployState, err := store.Load() @@ -241,3 +261,36 @@ func writeAudit(ev auditEvent) { Log().Warn("audit log write failed", "error", writeErr) } } + +// stateLockTimeout bounds how long a command waits for the state lock. +// Long enough that a normal deploy finishing up is simply waited out, +// short enough that a stuck or forgotten process is reported rather +// than leaving the caller staring at a hung terminal. +const stateLockTimeout = 30 * time.Second + +// acquireStateLock waits for the lock, but not forever. syscall.Flock +// with LOCK_EX blocks with no deadline, so the wait happens on a +// goroutine and the caller gives up after stateLockTimeout with an +// error that says what to do about it. +func acquireStateLock(lock *state.Lock) error { + done := make(chan error, 1) + go func() { done <- lock.Acquire() }() + + select { + case err := <-done: + if err != nil { + return exitWithError("LOCK_ERROR", output.ExitGeneralError, + "Failed to acquire state lock: "+err.Error(), + "Check if another trond process is running") + } + return nil + case <-time.After(stateLockTimeout): + // The goroutine keeps waiting and will release on process exit; + // the lock file is a shared resource, so abandoning the attempt + // is safe. + return exitWithError("LOCK_TIMEOUT", output.ExitGeneralError, + fmt.Sprintf("Another trond process has held the state lock for %s", stateLockTimeout), + "Find it with: ps aux | grep trond", + "A stuck process can be ended; the lock is released when it exits") + } +} diff --git a/cmd/restart.go b/cmd/restart.go index 013ff070..941ba574 100644 --- a/cmd/restart.go +++ b/cmd/restart.go @@ -28,7 +28,7 @@ func runRestart(cmd *cobra.Command, args []string) error { return err } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/rollback.go b/cmd/rollback.go index 889a6735..67644b1d 100644 --- a/cmd/rollback.go +++ b/cmd/rollback.go @@ -29,7 +29,7 @@ func runRollback(cmd *cobra.Command, args []string) error { return err } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/start.go b/cmd/start.go index 4d2a5844..fd7f32ff 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -28,7 +28,7 @@ func runStart(cmd *cobra.Command, args []string) error { return err } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/stop.go b/cmd/stop.go index 6dd41737..4a60b2d1 100644 --- a/cmd/stop.go +++ b/cmd/stop.go @@ -28,7 +28,7 @@ func runStop(cmd *cobra.Command, args []string) error { return err } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 044749d1..680bad8d 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -34,7 +34,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error { return err } - nc, err := resolveNodeContext(name) + nc, err := resolveNodeContextForWrite(name) if err != nil { return err } diff --git a/internal/mcp/conf_helpers.go b/internal/mcp/conf_helpers.go index 0a2a36b8..be152f09 100644 --- a/internal/mcp/conf_helpers.go +++ b/internal/mcp/conf_helpers.go @@ -3,7 +3,9 @@ package mcp import ( "context" "fmt" + "strings" + "github.com/tronprotocol/tron-deployment/internal/render" "github.com/tronprotocol/tron-deployment/internal/state" "github.com/tronprotocol/tron-deployment/internal/target" ) @@ -27,3 +29,16 @@ func readLiveConfigForMCP(ctx context.Context, tgt target.Target, node *state.Ma } return string(out), nil } + +// redactConfText removes witness signing keys from a whole conf before +// it is handed out. Split/join around render.RedactWitnessLines, which +// finds the key values by parsing rather than by line shape, so the +// formatting of the node's live conf does not matter. +func redactConfText(conf string) string { + if conf == "" { + return conf + } + // Splitting on \n and rejoining preserves \r\n line endings, since + // the \r stays attached to the line content. + return strings.Join(render.RedactWitnessLines(strings.Split(conf, "\n")), "\n") +} diff --git a/internal/mcp/conf_redaction_test.go b/internal/mcp/conf_redaction_test.go new file mode 100644 index 00000000..7d1e82bb --- /dev/null +++ b/internal/mcp/conf_redaction_test.go @@ -0,0 +1,40 @@ +package mcp + +import ( + "strings" + "testing" +) + +// trond://nodes/{name}/conf hands the whole conf to an MCP client and +// from there to a model provider. A witness node's conf carries its +// signing key, so nothing that reaches that resource may contain one. +func TestRedactConfTextRemovesWitnessKey(t *testing.T) { + const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + + for name, conf := range map[string]string{ + "multi line": "storage = {\n}\nlocalwitness = [\n " + key + "\n]\n", + "single line": `localwitness = ["` + key + `"]` + "\n", + "comment carries a bracket": "localwitness = [ # ] here\n " + key + "\n]\n", + } { + t.Run(name, func(t *testing.T) { + if got := redactConfText(conf); strings.Contains(got, key) { + t.Errorf("witness key survived redaction:\n%s", got) + } + }) + } +} + +// A conf with no key must come back untouched — the resource is how an +// agent reads a node's real configuration. +func TestRedactConfTextLeavesOrdinaryConfAlone(t *testing.T) { + conf := "storage = {\n db.engine = \"LEVELDB\"\n}\nnode.p2p.version = 11111\n" + if got := redactConfText(conf); got != conf { + t.Errorf("conf without a witness key was rewritten:\ngot %q\nwant %q", got, conf) + } +} + +func TestRedactConfTextEmpty(t *testing.T) { + if got := redactConfText(""); got != "" { + t.Errorf("empty conf became %q", got) + } +} diff --git a/internal/mcp/resources.go b/internal/mcp/resources.go index 8c10e624..00608123 100644 --- a/internal/mcp/resources.go +++ b/internal/mcp/resources.go @@ -186,11 +186,18 @@ func readNodeConfResource(ctx context.Context, req *mcp.ReadResourceRequest) (*m if err != nil { return nil, err } + // This resource hands the whole conf to the MCP client, and from + // there to a model provider — it is the one surface whose full text + // leaves the machine by design. A witness node's conf carries its + // signing key, so redact before it goes out. The drift tool reads + // the same helper and keeps the raw text, because it compares + // against a rendered config and a redacted side would report every + // witness node as drifted. return &mcp.ReadResourceResult{ Contents: []*mcp.ResourceContents{{ URI: req.Params.URI, MIMEType: "text/plain", - Text: live, + Text: redactConfText(live), }}, }, nil } diff --git a/internal/render/hocon.go b/internal/render/hocon.go index 303dd723..2942ee01 100644 --- a/internal/render/hocon.go +++ b/internal/render/hocon.go @@ -9,6 +9,8 @@ import ( "sort" "strings" + "github.com/gurkankaymak/hocon" + "github.com/tronprotocol/tron-deployment/internal/intent" "github.com/tronprotocol/tron-deployment/internal/security" ) @@ -135,12 +137,16 @@ func RedactWitnessLine(line string) string { // inside a multi-line `localwitness = [ ... ]` array. const redactedWitnessElement = `""` +// redactedValue stands in for a key value replaced in place, where the +// surrounding quoting and punctuation of the original line is kept. +const redactedValue = "" + // RedactWitnessLines redacts a whole config's worth of lines at once and // returns a slice of the same length, so callers can keep comparing the // raw lines positionally while emitting the redacted ones. // // It exists because RedactWitnessLine, looking at one line in isolation, -// cannot see the shape the shipped templates actually use: +// cannot see the shape the shipped templates use: // // localwitness = [ // @@ -152,26 +158,99 @@ const redactedWitnessElement = `""` // diff, verify-config and the MCP drift tool, whose output leaves the // machine) must go through this rather than mapping RedactWitnessLine // over the slice. +// +// The key values are found by parsing the config rather than by reading +// line shapes, so the formatting does not matter: single-line, +// multi-line, a `]` inside a comment on the opening line, an element +// that closes the array on its own line. A scan over line shapes gets +// each of those wrong, and for redaction the failure is silent — the +// caller cannot tell a config with no key from one whose key slipped +// through. When the text does not parse (a partial read off a node, a +// file mid-write) it falls back to the scan, which is why the scan is +// hardened rather than deleted. func RedactWitnessLines(lines []string) []string { + if values := witnessKeyValues(strings.Join(lines, "\n")); len(values) > 0 { + out := make([]string, len(lines)) + for i, line := range lines { + out[i] = redactValues(line, values) + } + return out + } + return redactWitnessLinesByScan(lines) +} + +// witnessKeyValues parses raw and returns the literal strings assigned to +// `localwitness`. Nil when the text does not parse or carries no key — +// the caller falls back to the scan, which cannot tell those two apart +// either but at least does not claim to. +func witnessKeyValues(raw string) []string { + cfg, err := hocon.ParseString(raw) + if err != nil || cfg == nil { + return nil + } + + // A parse error is reported through err, but this library also + // panics on some malformed input; a redaction path must not take + // the process down with it. + var values []string + func() { + defer func() { _ = recover() }() + for _, v := range cfg.GetArray(witnessKeyName) { + if v == nil { + continue + } + // String() quotes string values; the raw text may or may + // not have them, so match against both. + s := strings.TrimSpace(v.String()) + unquoted := strings.Trim(s, `"`) + if unquoted == "" { + continue + } + values = append(values, unquoted) + } + }() + return values +} + +// redactValues replaces every occurrence of a key value in line. Longest +// first, so one value being a prefix of another cannot leave a tail +// behind. +func redactValues(line string, values []string) string { + out := line + sorted := slices.Clone(values) + slices.SortFunc(sorted, func(a, b string) int { return len(b) - len(a) }) + for _, v := range sorted { + out = strings.ReplaceAll(out, v, redactedValue) + } + return out +} + +// redactWitnessLinesByScan is the fallback for text that does not parse. +// It walks the lines and stays inside an unterminated localwitness +// array. Comments are stripped before the bracket checks: a `]` inside a +// comment on the opening line would otherwise end the array before it +// began, and let the key through. +func redactWitnessLinesByScan(lines []string) []string { out := make([]string, len(lines)) inArray := false for i, line := range lines { - trimmed := strings.TrimSpace(line) + trimmed := stripComment(strings.TrimSpace(line)) switch { case inArray: - // The closing bracket carries no key material; anything - // before it does. if strings.HasPrefix(trimmed, "]") { inArray = false out[i] = line continue } - if trimmed == "" || strings.HasPrefix(trimmed, "#") || - strings.HasPrefix(trimmed, "//") { + if trimmed == "" { out[i] = line continue } out[i] = lineIndent(line) + redactedWitnessElement + // An element may close the array on its own line. + if strings.HasSuffix(trimmed, "]") { + inArray = false + } case IsWitnessKeyLine(line): out[i] = lineIndent(line) + redactedWitnessAssignment // An assignment that opens an array without closing it on @@ -186,6 +265,18 @@ func RedactWitnessLines(lines []string) []string { return out } +// stripComment removes a trailing HOCON comment so a `]` or `[` inside +// it cannot steer the bracket tracking. HOCON accepts both # and //. +func stripComment(s string) string { + if i := strings.Index(s, "#"); i >= 0 { + s = s[:i] + } + if i := strings.Index(s, "//"); i >= 0 { + s = s[:i] + } + return strings.TrimSpace(s) +} + // RenderHOCON loads the base template for the network and applies intent-driven overrides. // Returns the final HOCON config as a string. templateDir may be empty, in // which case the embedded template is used. diff --git a/internal/render/hocon_test.go b/internal/render/hocon_test.go index d295c8ce..e80b7141 100644 --- a/internal/render/hocon_test.go +++ b/internal/render/hocon_test.go @@ -1,6 +1,7 @@ package render import ( + "slices" "strings" "testing" @@ -575,3 +576,76 @@ func TestRedactWitnessLinesSingleLine(t *testing.T) { t.Fatalf("single-line key leaked: %q", got[0]) } } + +// Redaction must not depend on line shape. These are the shapes a scan +// over brackets gets wrong: a `]` inside the comment on the opening +// line ends the array before it began, and an element that closes the +// array leaves the scanner inside it for the rest of the file. +func TestRedactWitnessLinesIgnoresLineShape(t *testing.T) { + const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + + cases := map[string][]string{ + "comment carries a closing bracket": { + `localwitness = [ # note the ] in this comment`, + ` ` + key, + `]`, + `block = {`, + }, + "element closes the array": { + `localwitness = [`, + ` ` + key + `]`, + `node.p2p.version = 11111`, + }, + "single line": {`localwitness = ["` + key + `"]`}, + "multi line": {`localwitness = [`, ` ` + key + ` # matched`, `]`}, + } + + for name, in := range cases { + t.Run(name, func(t *testing.T) { + got := RedactWitnessLines(in) + if len(got) != len(in) { + t.Fatalf("length changed: got %d want %d", len(got), len(in)) + } + for i, line := range got { + if strings.Contains(line, key) { + t.Errorf("line %d leaked the key: %q", i, line) + } + } + }) + } +} + +// Over-redaction is its own failure: a diff whose unrelated lines all +// read tells the reader nothing. +func TestRedactWitnessLinesLeavesOtherKeysAlone(t *testing.T) { + const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + in := []string{ + `localwitness = [`, + ` ` + key + `]`, + `node.p2p.version = 11111`, + `storage.db.directory = "database"`, + } + got := RedactWitnessLines(in) + for _, want := range []string{`node.p2p.version = 11111`, `storage.db.directory = "database"`} { + if !slices.Contains(got, want) { + t.Errorf("unrelated line was rewritten; wanted %q in %q", want, got) + } + } +} + +// Text that does not parse still has to be redacted — that is the whole +// point of keeping the scan. +func TestRedactWitnessLinesFallsBackOnUnparseableText(t *testing.T) { + const key = "da146374a75310b9666e834ee4ad0866d6f4035967bfc76217c5a495fff9f0d0" + in := []string{ + `this { is not : valid ] hocon [`, + `localwitness = [`, + ` ` + key, + `]`, + } + for i, line := range RedactWitnessLines(in) { + if strings.Contains(line, key) { + t.Errorf("line %d leaked the key on the fallback path: %q", i, line) + } + } +}