Postgres Error Output

[stephannn]

Not a real PR, but the error variable is never used and when you use maybe an incorrect PostgreSQL version, as I do/did, you miss the real error about it. Alternative just a console.log would be sufficient.
I also put a RegEx to remove the password from the output

[si458]

@PTR-inc any comments to this PR?
i know its not a real PR, but makes a valid point about not knowing errors etc?

[PTR-inc]

The visible username/password dumpstring was removed after discussion #7221 and PR #7225.
So I absolutely see the usefulness, but revisiting this now I would suggest to put the extra infologging behind a debug option so it is intentional with an extra warning in the message.

Then the next question would be: is it still necessary to mask the username/password if you are intentionally debugging the db connection? If so, I would suggest to use a regex with the known password/username args (parent.args.postgres) to mask them. A lot safer should things change with the pgdump tool arguments and easier to read.

Also, where to output, the errorlog and/or the webui?

[DaanSelen]

The visible username/password dumpstring was removed after discussion #7221 and PR #7225. So I absolutely see the usefulness, but revisiting this now I would suggest to put the extra infologging behind a debug option so it is intentional with an extra warning in the message.

Then the next question would be: is it still necessary to mask the username/password if you are intentionally debugging the db connection? If so, I would suggest to use a regex with the known password/username args (parent.args.postgres) to mask them. A lot safer should things change with the pgdump tool arguments and easier to read.

Also, where to output, the errorlog and/or the webui?

May I ask why this was removed? Due to it exposing the username+password? If so then we should definitely put it behind some safeguards.

The debugging of password/username could be useful is things are not parsing well. But in that case I think people can manually add a console.log? I think its safest to mask it.

Put the errors in mesherrors.txt? Or where do you suggest?

[PTR-inc]

May I ask why this was removed? Due to it exposing the username+password? If so then we should definitely put it behind some safeguards.

Yes, if the backupcheck errored, it put the error unredacted to the webui, so in the case of postgres with user/pw.

The debugging of password/username could be useful is things are not parsing well. But in that case I think people can manually add a console.log? I think its safest to mask it.

Put the errors in mesherrors.txt? Or where do you suggest?

It can go to console.error for example. That way you can redirect it however you want through your server config.
The mentioned line currently returns it to the webui log so it shows up on the general tab of the 'my server' page. It would also be a nice option to message it to the serveradmin, like when you login in after a bad login, where there is a fly-out in the right upper corner, so you actively see there is an error. Now you have to check the log or the webui.

[stephannn]

I out-commented the func and just put a console.error for Maria, Mongo and Postgre. Maybe not the most beautiful way, but at least the error can be seen in the server console.

[DaanSelen]

I out-commented the func and just put a console.error for Maria, Mongo and Postgre. Maybe not the most beautiful way, but at least the error can be seen in the server console.

I think the whole resistance was due to the printing of the values to the front-end. So that would fix this I think.

[PTR-inc]

Maybe a bit overcautious, but maybe never log sensitive data unless specifically enabled with debug options?
For example, put it behind something like if (parent.config.settings.debug)
And/or use a string.replace on the user/password like: error.replace(parent.config.settings.postgres.password, "****")?

[stephannn]

I can change it by PostgreSQL to:
console.error("PostgreSQL DumpTool: " + error.toString().replace(/(postgresql:\/\/)([^:]+):([^@]+)@/g, '$1$2:****@') );
MongoDB has no password, so the error log as it is should be fine.
I am not sure about MariaDB. When no password is set, will it brake it? Can no password brake JavaScript?

or I can change it for maria and postgre like that:

if (typeof parent?.config?.settings?.postgres?.password === "string" && parent?.config?.settings?.postgres?.password.length > 0) {
                            error = error.replace(parent?.config?.settings?.postgres?.password, "****");
                        }
console.error("PostgreSQL DumpTool: " + error);

of course, in the config replace postgres with mariaDB

[PTR-inc]

Sorry for the late reply, hectic times over here, a few suggestions:

• For code readability I would suggest to not use the regexp and go for a postgresql://<username>:<password>@ replace pattern and don't forget to encodeURIComponent them before the replace.
• MongodDB can use username/password, see here. Mongodump allows for arguments (mongodump --username joe --password secret1 mongodb://mongodb0.example.com:27017 --ssl) or a connectionstring (mongodump --uri="mongodb://[username:password@]host1[:port1][,host2[:port2],...[,hostN[:portN]]][/[database][?options]]"), so there is a lot more to potentially check.
• Maria/mysql use the --user/--password command line variant for passing them (see here). They allow for an empty password, but string.replace with an empty pattern does nothing, so that shouldn't be an issue. And like postgres variant, use the full argument, with the --user/--password added to the replace pattern, taking this into account.
• Add the check to only output if debug enabled
• Maybe redundant, but be sure to test edge cases like all same username/password/db combo's, empty password, special characters etc.

All this is because of the db backup being done with the different external tools (also see all the issues with not finding the dump tool because of path/install/version differences). This could all be much safer if the reliance on those tools could be removed. I have looked for options/solutions (not exhaustive, and a year ago), but found none without needing extensive rewriting.

[stephannn]

@PTR-inc thanks for the explanation. I just had an issue with AppLocker and encodeURIComponent. Adobe used quotation marks in their certificate name which breaks the event log with %22
Anyways, and another try :D

[PTR-inc]

Yeah, special characters/diacritic marks are always a nice testcase. And after that non-english OSes :-)

Looks good, only question I have at the moment is regarding the encodeURIComponent(processedError) part. The password is URIencoded before being added to the dumptool command line. I assume (as I am able to check it myself at the moment) the error will contain the already URIencoded password. For the proper compare, you would then need to do something like encodeURIComponent(parent.config.settings.postgres.password).

[stephannn]

you are absolutely right, not sure if just moving the encoding part outside is enough:

`const testCases = [
    {
        name: "Auth error with password",
        error: "Access denied for user 'backup' using password '%pwd%'",
        password: 'Super"Secret123!'
    },
    {
        name: "Connection string contains password",
        error: "Could not connect using postgresql://user:%pwd%@dbserver:5432/db",
        password: "MyP@ssw0rd"
    },
    {
        name: "Password appears multiple times",
        error: "Password test123 rejected. Authentication failed for password test123.",
        password: "test123"
    },
    {
        name: "No password configured",
        error: "Unknown host dbserver",
        password: ""
    },
    {
        name: "Special characters and newlines",
        error: "Dump failed:\nERROR 1045 (28000): Access denied",
        password: "secret"
    }
];

const parent = {
    config: {
        settings: {
            postgres: {
                password: ""
            }
        }
    },
    debug: (category, message) => {
        console.log(`[${category}] ${message}`);
    }
};

for (const testCase of testCases) {
    
    parent.config.settings.postgres.password = testCase.password;
    let processedError = testCase.error.replaceAll("%pwd%", testCase.password);

    console.log(`\n--- Test case: ${testCase.name} ---`);
    console.log("Password: " +  testCase.password );
    console.log("Error: " + processedError);

    if (typeof parent?.config?.settings?.postgres?.password === "string" &&	parent.config.settings.postgres.password.length > 0) {
							processedError = encodeURIComponent(processedError.replaceAll(parent.config.settings.postgres.password, "****"));
		}

    parent.debug(
        "backup", "Test: " + processedError
    );
}
`