lightningd: try harder to ensure uniqueness in --dev-save-plugin-io names.
What changed, and why it matters
This commit fixes a minor developer-only bug where a special diagnostic flag (--dev-save-plugin-io) could reuse filenames across internal restarts, causing file creation to fail. It adds a startup timestamp to filenames so they stay unique. This flag is not used in production and the failure mode is a noisy error, not a security vulnerability.
No security action required. Treat as a normal code-quality/test fix. If running with --dev-save-plugin-io during internal restart tests, update to this commit to avoid spurious failures.
Security signals we found
Fixes a filename collision in a developer-only diagnostic feature
Uses O_EXCL to prevent overwriting existing files
No privilege boundary crossed; no untrusted input used in filename
No cryptographic, authentication, or network implications
Evidence from the diff
The dev_save_plugin_io() function in lightningd/plugin.c now records a static starttime from time_mono() on first call and includes it in saved plugin I/O filenames alongside type, name, PID, and a counter. This prevents O_CREAT|O_EXCL collisions when lightningd re-executes itself (e.g., during version upgrade or recovery tests) because the previous scheme used only pid+counter, which could repeat after restart. Two tests that previously deleted the dev-save-plugin-io option to avoid this collision are simplified, confirming the fix.
Changed components
lightningd/plugin.c dev_save_plugin_io()tests/test_misc.pyInspect captured patch +10 / −8
diff --git a/lightningd/plugin.c b/lightningd/plugin.c
index 41a2d61e..98637642 100644
--- a/lightningd/plugin.c
+++ b/lightningd/plugin.c
@@ -2622,16 +2622,24 @@ static void dev_save_plugin_io(struct plugins *plugins,
const char *buf, size_t len)
{
static size_t counter;
+ static u64 starttime;
const char *file;
int fd;
if (!plugins->dev_save_io)
return;
+ /* If we reexec, we still want unique names */
+ if (!starttime) {
+ struct timemono start = time_mono();
+ starttime = start.ts.tv_sec * 1000000 + start.ts.tv_nsec / 1000;
+ }
+
file = path_join(tmpctx, plugins->dev_save_io,
- take(tal_fmt(NULL, "%s-%s-%u-%zu",
+ take(tal_fmt(NULL, "%s-%s-%u-%"PRIu64"-%zu",
type, name,
(unsigned int)getpid(),
+ starttime,
counter++)));
fd = open(file, O_CREAT|O_EXCL|O_WRONLY, 0600);
if (fd < 0 || !write_all(fd, buf, len))
diff --git a/tests/test_misc.py b/tests/test_misc.py
index dc5e0a9f..e98c132d 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -3692,8 +3692,6 @@ def test_version_reexec(node_factory, bitcoind):
# We use a file to tell our openingd wrapper where the real one is
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "openingd-real"), 'w') as f:
f.write(os.path.abspath('lightningd/lightning_openingd'))
- # Internal restart doesn't work well with --dev-save-plugin-io
- del l1.daemon.opts['dev-save-plugin-io']
l1.start()
# This is a "version" message
verfile = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "openingd-version")
@@ -4569,11 +4567,7 @@ def test_setconfig_changed(node_factory, bitcoind):
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "deletes database, which is assumed sqlite3")
def test_recover_command(node_factory, bitcoind):
- l1 = node_factory.get_node(start=False)
- # Internal restart doesn't work well with --dev-save-plugin-io
- del l1.daemon.opts['dev-save-plugin-io']
- l1.start()
- l2 = node_factory.get_node()
+ l1, l2 = node_factory.get_nodes(2)
l1oldid = l1.info['id']
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.