listconfigs shows only 80 chars of "lightning-dir" path

[SimonVrouwe]

On latest master (c4203e7), test_relative_config_dir fails when lightning-dir directory path is longer then 80 characters.

Accidentally ran into this when using the $TEST_DIR variable for the pytests, below example results in --lightning-dir path to be 81 chars.

TEST_DIR=/tmp/aaaaaaaaaaaaaaaaaa VALGRIND=0 pytest tests/ -k 'test_relative_config_dir'
============================================================================================================================= test session starts =============================================================================================================================
platform linux -- Python 3.9.2, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/simon/git/SimonVrouwe/lightning, configfile: pytest.ini
plugins: custom-exit-code-0.3.0, test-groups-1.0.3, xdist-2.5.0, forked-1.4.0, timeout-2.1.0
collected 528 items / 527 deselected / 1 selected                                                                                                                                                                                                                             

tests/test_misc.py F                                                                                                                                                                                                                                                    [100%]

================================================================================================================================== FAILURES ===================================================================================================================================
__________________________________________________________________________________________________________________________ test_relative_config_dir ___________________________________________________________________________________________________________________________

node_factory = <pyln.testing.utils.NodeFactory object at 0x7fdd97606d60>

    def test_relative_config_dir(node_factory):
        l1 = node_factory.get_node(start=False)
        initial_dir = os.getcwd()
        lndir = l1.daemon.opts.get("lightning-dir")[:-1]
        *root_dir, l1.daemon.opts["lightning-dir"] = lndir.split('/')
        os.chdir('/'.join(root_dir))
        l1.daemon.executable = os.path.join(initial_dir, l1.daemon.executable)
        l1.start()
>       assert os.path.isabs(l1.rpc.listconfigs()["lightning-dir"])
E       AssertionError: assert False
E        +  where False = <function isabs at 0x7fdd9d7cf820>('tmp/aaaaaaaaaaaaaaaaaa/ltests-nhzau93b/test_relative_config_dir_1/lightning-1...')
E        +    where <function isabs at 0x7fdd9d7cf820> = <module 'posixpath' from '/usr/lib/python3.9/posixpath.py'>.isabs
E        +      where <module 'posixpath' from '/usr/lib/python3.9/posixpath.py'> = os.path

/home/simon/git/SimonVrouwe/lightning/tests/test_misc.py:1851: AssertionError

Seems caused by the way listconfigs calls opt->show(), which in this case is opt_show_charp() which only copies 80 chars.

lightning/lightningd/options.c

Lines 1568 to 1569 in c4203e7

	opt->show(buf, opt->u.carg);
	strcpy(buf + OPT_SHOW_LEN - 1, "...");

BTW listconfigs and getinfo returns different lightning-dir <== not documented very well

[kevkevinpal]

this is the log I get when running
make -j$(nproc) TEST_DIR=/tmp/aaaaaaaaaaaaaaaaaa VALGRIND=0 pytest tests/ -k 'test_relative_config_dir' >> test.txt

https://pastebin.com/f5NFsJrW

when I run this it works fine/starts to run the tests
make -j$(nproc) TEST_DIR=/tmp/aa VALGRIND=0 pytest tests/ -k 'test_relative_config_dir' >> test.txt

I will try and dig deeper

[kevkevinpal]

I see this error in the log I posted above
OSError: AF_UNIX path too long

[kevkevinpal]

Would it make sense to just write a check to see what the TEST_DIR length is and throw an error if it is above a certain length?

[kevkevinpal]

opened a PR I just added an if statement after TEST_DIR is defined and then threw an error if the path was above 12 characters

[SimonVrouwe]

@kevkevinpal
I am not sure how unsafe it is that listconfigs shows an incomplete `lightning-dir" ending in 3 dots like

"lightning-dir": "/tmp/aaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbSP...",

and is probably rare. But the fact it fail above test tells us we better fix it ?

A temporary solution, abort lightningd startup if the length of the path is >76 (above that the dots appear).
Something like:

diff --git a/common/configdir.c b/common/configdir.c
index c03abe18c..2a92d8f36 100644
--- a/common/configdir.c
+++ b/common/configdir.c
@@ -23,11 +23,15 @@ char *opt_set_talstr(const char *arg, char **p)
        return opt_set_charp(tal_strdup(options_ctx, arg), p);
 }
 
-static char *opt_set_abspath(const char *arg, char **p)
+static char *opt_set_abspath_max76(const char *arg, char **p)
 {
+       char* ret;
        tal_free(*p);
-       return opt_set_charp(path_join(options_ctx, take(path_cwd(NULL)), arg),
-                            p);
+       ret = opt_set_charp(path_join(options_ctx, take(path_cwd(NULL)), arg),p);
+       if (strlen(*p) > 76)
+               errx(1, "absolute path of command line option exceeds 76 chars: '%s'",
+                                    *p);
+       return ret;
 }
 
 /* Tal wrappers for opt. */
@@ -302,14 +306,14 @@ void initial_config_opts(const tal_t *ctx,
        /* First, they could specify a config, which specifies a lightning dir
         * or a network. */
        *config_filename = NULL;
-       opt_register_early_arg("--conf=<file>", opt_set_abspath, NULL,
+       opt_register_early_arg("--conf=<file>", opt_set_abspath_max76, NULL,
                               config_filename,
                               "Specify configuration file");
 
        /* Cmdline can also set lightning-dir. */
        *config_basedir = NULL;
        opt_register_early_arg("--lightning-dir=<dir>",
-                              opt_set_abspath, NULL,
+                              opt_set_abspath_max76, NULL,
                               config_basedir,
                               "Set base directory: network-specific subdirectory is under here");
 
@@ -335,7 +339,7 @@ void initial_config_opts(const tal_t *ctx,
                                       "Set base directory: network-specific subdirectory is under here");
        } else {
                opt_register_early_arg("--lightning-dir=<dir>",
-                                      opt_set_abspath, NULL,
+                                      opt_set_abspath_max76, NULL,
                                       config_basedir,
                                       "Set base directory: network-specific subdirectory is under here");
        }