fix(security): stop leaking the witness private key through output and file modes - #202
Merged
kuny0707 merged 6 commits intoAug 3, 2026
Conversation
RenderHOCON read the witness private key from the environment and inlined it into the single string every caller treats as ordinary text, so the key reached stdout, the -o json payload, --output-dir files, the four diff paths and both MCP tool results. Split the render into a display form carrying <REDACTED:ENV_NAME> and an unexported deploy form reachable only via Deployable(). Deploy paths use the deploy form; preview paths use the display form. The four positional differs still compare the real bytes, so drift detection is unchanged, but every emitted line - including --context neighbours - passes through a stateless single-line redactor. config render now signals redaction with a stderr warning and a redacted field. Deployed bytes and config_hash are byte-identical to before.
config render --output-dir persisted the rendered .conf with os.WriteFile at 0644 inside a 0755 directory, so any local account could read a witness key or any secret carried in config_overrides. The file is now opened O_WRONLY|O_CREATE|O_TRUNC at 0600 and fchmod'ed on the open descriptor before the body is written, so it lands at 0600 even when the file already existed at 0644 - which is the operator who has the bug today. The output directory is created 0700; a pre-existing directory keeps its own mode. Compose and systemd artifacts stay 0644 as trond never resolves a secret into them.
The docker runtime wrote the rendered node config - which for a witness node carries the inlined signing key - with mode 0644 into a directory created by a bare mkdir -p. On an SSH target or a shared state dir every local account could read it. The config is now written 0600 and the deployment directory created with mkdir -p -m 0700, so the mode is set at creation rather than by a follow-up chmod on a path that could have been substituted. No ownership transfer is needed: the compose bind mount is read by the container as root, confirmed against the image config, which declares no USER.
…o it generate_witness_key wrote the witness private key to .shadow-fork-witness.env with a plain shell redirect, so the file was created world-readable under the default umask. The caller-supplied branch never fixed the mode at all, and the self-generated branch chmod'ed only after python3 had finished writing - the key was readable on disk for the duration of the write. Both branches now go through init_key_stash, which unlinks any existing path and creates the file empty inside a (umask 077) subshell, so the key only ever lands on a 0600 file. This also stops the script writing the key through a symlink planted at that path.
…utput dir WriteAddressList persisted one freshly minted secp256k1 private key per receiver - 1000 by default, 100000 in the README example - through os.Create, so the CSV was created world-readable inside a 0755 directory. The sibling secret path in the same tool already used 0600. The CSV is now opened O_WRONLY|O_CREATE|O_TRUNC at 0600 and fchmod'ed on the open descriptor before the first row, so a file left at 0644 by an earlier run is tightened too. The output directory is created 0700 when txgen creates it. txgen-output/ is added to .gitignore so the key dump cannot be committed by accident. The generated transaction CSVs, broadcast outputs and statistics report keep the default mode - they carry txIDs, signed-transaction JSON and public keys, never private key material.
golangci-lint's unparam flagged witnessIntent's envName, which every call site passed the same literal. Hoisted to a package-level constant.
barbatos2011
force-pushed
the
security/witness-key-exposure
branch
from
August 3, 2026 07:54
be29a92 to
b46fb09
Compare
barbatos2011
added a commit
to barbatos2011/tron-deployment
that referenced
this pull request
Aug 5, 2026
The multi-node path hand-rolled render + Deploy + state and had drifted
from the core it was meant to mirror:
- it never imported internal/build, so a node with `build:` rendered an
empty image: line and deployed nothing usable — silently, with a green
config validate
- it hardcoded jdk=17 for JVM arg selection instead of probing the target
- it hardcoded the docker runtime instead of honouring target.runtime
nodeIntent projects the network intent down to the single-node form Apply
consumes. Apply keys the compose project, the state entry and Result.Name
off Intent.Name and reads only Nodes[0], so the projection renames as well
as slices. The hash is computed over the projected intent, not the network
file, so editing one node redeploys that node and leaves its siblings at
no_change — a shared hash would make node 1 look unchanged the moment node
0 had been applied.
Two things had to be fixed in Apply first, both found by doing the
refactor rather than by reading it:
- Apply never wrote P2PPort. network add builds the joining node's peer
list from the P2PPort of every node in state and SKIPS zero entries, so
routing create through Apply as-is would have left every node
unreachable as a peer and the late joiner permanently isolated — with
nothing in either command's output to say why.
- monitoring would have been deployed once per node. RenderHOCON keys the
metrics auto-enable off Intent.Monitoring so the field cannot simply be
cleared; SkipMonitoring suppresses the deploy and leaves the field
alone. Without it the last node's stack wins and scrapes exactly one
node, which looks healthy and is not.
Two consequences of create becoming an Apply caller, both wanted:
- it inherits RenderHOCONWithSecrets (tronprotocol#202), so the witness key is no
longer inlined by a second copy of that call kept in sync by hand
- it now passes guard.Requested() as RequirePrivate, so the state-based
half of the gate (tronprotocol#203) applies here too. guard.Enforce above only
sees the intent's network LABEL; the core also checks the network
recorded in state for a node already deployed under the same name.
Restores "network create" to the list of callers that inherit it.
Validated against a real Docker daemon: make e2e passes, including
TestE2E_Network_PrivateLifecycle (2 containers, create → status → destroy).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five fixes for witness/SR private key exposure, found by an audit of
developat0c654cd. Each commit stands alone; they are grouped because they all concern key material reaching somewhere it should not.What is fixed
stop resolving the witness key into rendered config text— the root cause.renderHOCONAppendixread the key from the environment and inlined it into the one string every caller treats as ordinary text, so it reached stdout, the-o jsonpayload,--output-dirfiles,plan --diff,config diff,verify-configand both MCP tool results. The render is now split into a display form carrying<REDACTED:ENV_NAME>and an unexported deploy form reachable only throughDeployable().The four differs are positional and LCS-free, so any line-count change above
localwitnessmisaligns them and emits the key — they still compare the real bytes, so drift detection is unchanged, but every emitted line including--contextneighbours now passes through a stateless single-line redactor. Deployed bytes andconfig_hashare byte-identical to before, verified by sha256 across mainnet/nile/private with and without seeds, discovery,config_overridesandaccount_address.write rendered config files 0600—config render --output-dirwrote the.confat 0644 in a 0755 directory. Now openedO_WRONLY|O_CREATE|O_TRUNCat 0600 with anfchmodon the open descriptor before the body, so a file an earlier run left at 0644 is tightened too.write docker node config 0600 in a 0700 deployment dir— same exposure on the docker runtime. No ownership transfer is added: the bind mount is read by the container as root, confirmed against the published image config, which declares noUSER.create the shadow-fork key stash 0600 before writing to it— the caller-supplied branch never set the mode, and the self-generated branch chmod'ed only afterpython3had finished writing, leaving the key readable mid-write. Also stops the script writing the key through a symlink planted at that path.write the txgen receivers CSV 0600— one private key per receiver, 1000 by default, was written world-readable.txgen-output/added to.gitignore.Behaviour changes worth reviewing
config renderand the MCPconfig_rendertool no longer return the real key.--output-dirtherefore produces a preview java-tron will reject; this is signalled by a stderr warning and aredactedfield rather than failing silently.localwitness = ["<REDACTED>"]in place of anylocalwitnessline. Counts andin_syncare unchanged.config-render.schema.json, soSchemaVersiongoes 1.12.2 → 1.12.3 with a regenerated baseline.Testing
go test ./... -race -count=1,go vet ./...andgofmtare clean on the branch. Each fix ships regression tests that fail against the unpatched tree.Note on ordering
Two sibling PRs also bump
SchemaVersionto 1.12.3 independently. Whichever merges second will need a rebase and a bump to 1.12.4 withmake snapshot-schema-baseline.