Skip to content

feat(cli): support nested maps in rc config file - #9578

Merged
nflaig merged 20 commits into
ChainSafe:unstablefrom
lodekeeper:feat/nested-rc-config
Jul 5, 2026
Merged

feat(cli): support nested maps in rc config file#9578
nflaig merged 20 commits into
ChainSafe:unstablefrom
lodekeeper:feat/nested-rc-config

Conversation

@lodekeeper

@lodekeeper lodekeeper commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Motivation

Lodestar's --rcConfig file currently requires nested options to be written as dotted keys:

rest.address: "0.0.0.0"
rest.port: 9596

This surprised a user who expected the idiomatic nested YAML form (rest:address:). The nested form is nicer, but we don't want to break existing dotted configs — so this PR supports both.

Description

Nested maps → dotted keys

Nested maps in the config file are flattened to dotted keys before being handed to yargs, so both of these produce identical results:

# dotted (still works)
rest.address: "0.0.0.0"
rest.port: 9596

# nested (now works too)
rest:
  address: "0.0.0.0"
  port: 9596

Why flattening: cli.ts sets parserConfiguration({"dot-notation": false}) (needed so .strict() keeps working with dotted option names), so options are registered as literal dotted keys ("rest.address") and yargs .config() only matches literal dotted keys from the file — nested maps are silently ignored today. Flattening the parsed file in the rcConfig read callback bridges the gap without touching parser config or the strictness guarantees.

  • Arrays are preserved as values (not flattened by index) so array options (rest.namespace, bootnodes, …) keep working.
  • Already-dotted keys pass through unchanged → fully backward compatible.
  • Prototype-pollution safe: __proto__ is skipped and non-plain input returns {}.
  • If an option is given in both nested and dotted form, the value appearing last in the file wins.
  • Applies to every command that shares the global --rcConfig option (beacon / validator / bootnode).

.enabled → bare on/off flag

On/off options are registered as a bare boolean flag (e.g. --metrics), which reads awkwardly as a nested map. After flattening, {prefix}.enabled is rewritten to the bare {prefix} flag so the natural nested form works:

metrics:
  enabled: true   # → the --metrics flag
  port: 8008

metrics.enabled: truemetrics: true. If both {prefix} and {prefix}.enabled are present, .enabled is left as-is so the conflict surfaces at yargs' strict check instead of being silently swallowed.

Testing

  • flattenObject — 13 unit tests: nested → dotted, dotted pass-through, mixed nested+dotted, array preservation, arrays-of-objects, primitive/null values, deep nesting, top-level scalars, empty-map elision, nested/dotted collision precedence, __proto__ guard, non-plain input, and non-plain object leaves (Date).
  • translateEnabledKeys — 7 unit tests: .enabled → bare flag, the nested metrics: {enabled, port} form, multiple prefixes, the already-set conflict case, no-op when absent, a top-level enabled is not translated, and deeply-prefixed keys.
  • check-types, biome lint, full build, and the existing cli util / options / beacon unit suites pass — no regressions.

Docs

Adds a compact example to the --rcConfig option showing both forms (a nested map and a dotted key), so it renders in the generated CLI reference (beacon-cli#--rcconfig). Because --rcConfig is a global option, its example renders in every command's docs, so it uses options common to all of them (network / logLevel / metrics) rather than a beacon-only option.

Rendering an option example that's a config snippet (no shell command) needed a small docsgen change: renderOption renders the example description directly, and CliOptionDefinition.example becomes description-only (Omit<CliExample, "title" | "command">). The example is docs-only (yargs ignores it, so --help is unchanged) and the generated *-cli.md files are gitignored.

This supersedes the dedicated docs page in #9574 (now closed) — a whole page was overkill; an inline example is enough.


🤖 Generated with AI assistance

The --rcConfig file required nested options to be written as dotted keys
(e.g. rest.address). Flatten nested maps to dotted keys before handing the
parsed file to yargs, so both nested maps and dotted keys are accepted.
Arrays are preserved (not flattened by index) and already-dotted keys pass
through unchanged, keeping existing configs fully backward compatible.

🤖 Generated with AI assistance
@lodekeeper
lodekeeper requested a review from a team as a code owner July 2, 2026 09:35

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a flattenObject utility to flatten nested configuration objects into dot-notation keys, enabling CLI configuration files to support both nested and dotted formats. The review feedback highlights critical security and robustness issues in the implementation, specifically pointing out a prototype pollution vulnerability, incorrect handling of non-plain objects (like Date or RegExp), and potential runtime errors if the input is not a plain object. A revised implementation with an isPlainObject helper is suggested to resolve these concerns.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/cli/src/util/object.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb4815f069

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/cli/src/util/object.ts Outdated
Add a small nested-map example to the --rcConfig option so it renders in the
generated CLI reference (beacon-cli#--rcconfig), rather than a dedicated docs
page. The example field is docs-only and does not affect --help output.

🤖 Generated with AI assistance
@nflaig

nflaig commented Jul 2, 2026

Copy link
Copy Markdown
Member

@lodekeeper please take a look at the bot reviews

…ing inputs

Address Gemini review on ChainSafe#9578: guard flattenObject with isPlainObject (reused
from @lodestar/utils) so non-object input (an empty/scalar rc config makes
readFile return undefined/a primitive) returns {} instead of throwing on
Object.entries, and non-plain object values (Date/Buffer/typed arrays/arrays)
are kept as leaf values rather than silently dropped or mis-flattened. Skip the
__proto__ key as prototype-pollution defense-in-depth; objects whose own
`constructor` shadows the function are already rejected by isPlainObject. Adds
regression tests for all three cases.

🤖 Generated with AI assistance
Address Codex review on ChainSafe#9578: an option that is both a value/flag and a prefix
(e.g. network, metrics, builder) can't express both the scalar and its
sub-options under one nested YAML key; document that those are set with dotted
keys. flattenObject itself is unchanged (it faithfully flattens valid input).

🤖 Generated with AI assistance
@lodekeeper

Copy link
Copy Markdown
Contributor Author

Addressed the bot reviews on #9578:

  • Gemini (prototype pollution / robustness): hardened flattenObject with isPlainObject (reused from @lodestar/utils) — non-object input returns {} instead of throwing, non-plain objects (Date/Buffer/etc.) are kept as leaves, and __proto__ is skipped. +3 regression tests (7f8349a).
  • Codex (prefix collisions): not a flattenObject bug — it faithfully flattens valid input and never drops a provided value; the nested form just can't express both a scalar and its sub-options for flag/prefix options (network/metrics/builder), which is inherent to YAML. Documented the caveat on the --rcConfig example (6900079), pointing to the dotted form for those.

Green locally: biome, check-types, 13/13 flattenObject tests.

Comment thread packages/cli/src/options/globalOptions.ts
Nico found the previous Note unclear. Rewrite it around a concrete case (the
metrics enable flag vs its metrics.port sub-option) and show the dotted-key form
in a yaml block, since YAML can't give one key both a value and nested children.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/cli/src/options/globalOptions.ts Outdated
Translate `{prefix}.enabled` -> `{prefix}` after flattening the rc config, so on/off
flags whose internal config nests the toggle under `.enabled` (metrics, rest, builder)
can be written in the natural nested form:

  metrics:
    enabled: true
    port: 8008

No CLI option is registered with an `.enabled` suffix, so the rewrite is unambiguous; a
bare `{prefix}` already set (mixed dotted/nested) is left as a conflict. Updates the
rcConfig docs example to the nested form (drops the dotted-key caveat) and adds unit
tests for the translation.

Addresses nflaig review on ChainSafe#9578.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/cli/src/options/globalOptions.ts Outdated
Comment thread packages/cli/src/options/globalOptions.ts Outdated
Comment thread packages/cli/src/options/globalOptions.ts Outdated
Comment thread packages/cli/src/options/globalOptions.ts Outdated
@nflaig

nflaig commented Jul 4, 2026

Copy link
Copy Markdown
Member

@lodekeeper see my comments

Address nflaig review on ChainSafe#9578:
- drop the flag/prefix Note (the example is sufficient)
- remove the redundant example lead-in; fold the "nested maps or dotted keys"
  hint into the option description instead
- drop the example command line and the config file name from the yaml

Make the option-level `example.command` optional (rcConfig is the only option
with an example, and it is illustrative config with no command to show) and skip
the command block in docsgen when it is absent.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@nflaig

nflaig commented Jul 4, 2026

Copy link
Copy Markdown
Member

@lodekeeper it's really messed up now

image

please just keep this PR minimal...

@nflaig

nflaig commented Jul 4, 2026

Copy link
Copy Markdown
Member

@lodekeeper

The trimmed rcConfig example started with a ```yaml fence, but renderOption
emits the example body inline after "example: ", so the fence opener landed
mid-line ("example: ```yaml") and MDX stopped treating the page as markdown —
every option below rcConfig rendered as raw source.

- add a one-line lead-in before the yaml block so the fence starts at line
  start again (renders correctly)
- restore the example command per review
- revert the renderExampleBody / CliExample.command-optional change; no longer
  needed once the command is back, and keeps the PR diff minimal

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lodekeeper

Copy link
Copy Markdown
Contributor Author

Fixed in 440318e.

Root cause: the trimmed example started with a ```yaml fence, but docsgen renders the example inline after example: , so it came out as example: ```yaml (fence mid-line) → MDX stopped parsing the page and everything below --rcConfig rendered as raw source.

Fix: a one-line lead-in before the yaml so the fence starts at line-start (verified with docs:build), restored the command, and reverted the renderExampleBody / CliExample changes from the previous push — not needed with the command back, so the diff is down to just the feature + the example (docsgen + utils untouched).

lodekeeper and others added 2 commits July 4, 2026 20:55
Drop the "example:" label and the command line from the rcConfig docs so it
shows just the config snippet (per review). docsgen now renders an option
example that has no command as a standalone block instead of forcing an inline
"example: " prefix, which also keeps the code fence at line start.

- rcConfig example is the yaml snippet only (no lead-in, no command)
- renderExampleBody handles command-less examples
- CliOptionDefinition.example allows an optional command

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review, don't modify docsgen for a single CLI flag — too invasive for the
scope. Revert 2f496f0: the rcConfig example keeps its lead-in and command so
it renders through the existing docsgen path unchanged; markdown.ts/command.ts
are back to base.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/cli/src/options/globalOptions.ts Outdated
lodekeeper and others added 5 commits July 4, 2026 21:39
Per review (r3523885164): hyphenated filename so the example doesn't visually
conflate the file name with the dotted option keys the feature introduces;
also matches the existing rc-config.yml / rcconfig.yml docs examples.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review: keep the command in the example, but render it as a clean standalone
block (yaml + command) without the inline "example:" label or lead-in text.
Minimal docsgen change — renderOption no longer prefixes an option example with
"example: "; rcConfig is the only option with an example.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "example:" keyword isn't worth a docsgen change — revert c8a47f1 so docsgen
stays at base. rcConfig keeps the command + lead-in + "example:" label, rendering
through the existing path. PR back to feature + example only.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The command line was the noisy part; keep the "example:" label + yaml example
but drop the command. Minimal docsgen change: example.command is now optional
and renderExampleBody skips the sh block when absent; renderOption still labels
the block "example:". rcConfig is the only option with an example.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rcConfig is a global option, so its example renders in every command's CLI docs
(beacon, validator, bootnode). The previous example used `rest.*`, which is
beacon-only and makes no sense in the validator/bootnode reference. Use options
available in all commands (network, logLevel, metrics) — still demoing a nested
map + the .enabled translation via metrics.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
No CLI option uses example.command — rcConfig is the only option with an
example, and it's a config snippet with no command. Remove command from the
option-example type, render the description directly in renderOption, and
restore renderExampleBody to its original form (now used only by command-level
examples). Docs output unchanged.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@nflaig nflaig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lodekeeper few more comments, otherwise looks good, please once you applied the remaining changes, push another update to your github pages and verify it looks good on the rendered docs

Comment thread packages/cli/src/options/globalOptions.ts Outdated
Comment thread packages/cli/docsgen/markdown.ts
Comment thread packages/cli/src/options/globalOptions.ts Outdated
Address nflaig review (r3524527982): the description says options can be written
as nested maps or dotted keys, but the example only showed the nested form.
Demonstrate both — `metrics` as a nested map and `metrics.port` as a dotted key —
and drop the trailing colon per the suggestion.

🤖 Generated with AI assistance

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/cli/src/options/globalOptions.ts Outdated
🤖 Generated with AI assistance
@nflaig
nflaig merged commit c0e6c9e into ChainSafe:unstable Jul 5, 2026
19 checks passed
@wemeetagain

Copy link
Copy Markdown
Member

🎉 This PR is included in v1.45.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants