Fixes: #2536.
Multiple fixes:
- When miniflux and postgresql are install simultaneously, miniflux setup may be
installed before postgresql is started.
- When postgresql is already installed and disabled (due to a previous
uninstall), then postgresql may not be running during miniflux package
installation (and fail initial DB setup).
- When app is being installed while it is disabled, the database may not running
and may lead to failure in removing the app database.
Tests:
- Run functional tests on stable/testing twice in a row.
- Install the app without postgresql or miniflux installed.
- Disable the app and uninstall it. DB is purged.
- Uninstall and re-install (with postgresql is disabled during installed).
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Tests:
- Uninstall miniflux and postgresql. Install freshly with all the patches in
this series. When installing miniflux freshly, postgresql is not disabled soon
after miniflux package is installed. Without this patch, postgresql is disabled
after packages are installed leading to a setup failure.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
This prevents logging usernames and passwords to the journal logs and to the
Git repo configuration. Also, avoids usernames and passwords appear in the
process list when cloning a repository.
Tests performed:
- Create a new repository by cloning an existing repository URL with basic
auth credentials. Check that:
- Cloning succeeds.
- Journal logs don't contain URLs with credential info.
- The configuration of the cloned repository doesn't contain credential info.
- Try to clone a non-existing repository URL that contains credential
info. Cloning fails and there are no credential info in the journal logs.
- Cloning a public git repository without credential info succeeds.
- All the gitweb module tests pass.
Signed-off-by: Veiko Aasa <veiko17@disroot.org>
[sunil: Add/fix some more type hints]
[sunil: Add tests for URL parsing]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>
- These situation occur when server encounters an error when trying to formulate
a response. All exceptions during execution of actions are caught and reported
properly. However, server may encounter errors during processing of exception
raised in an action. Or may die abruptly. This special error will make
identifying such situations easier.
Tests:
- Add a 'return' after _read_request() in
privileged_daemon.py:RequestHandler:handle(). This will trigger this error.
Starting FreedomBox service will show these errors as 'ConnectionError: Server
returned empty response'. Similarly running 'freedombox-cmd --no-args plinth
is_package_manager_busy' will show the same error.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- Now that we have a mechanism for properly collecting, transmitting, and display
the stdout and stderr. There is no reason not to collect all of the stdin and
stderr.
- Also, the stdin/stderr=subprocess.PIPE is redundant and prevents the output
from getting collected for debugging. So, remove it.
Tests:
- Ran functional tests on backups, calibre, ejabberd, email, gitweb, ikiwiki,
infinoted, kiwix, mediawiki, mumble, nextcloud,, openvpn, samba, wireguard,
zoph. 2-3 issues were found but did not seem like new errors.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This is to capture stdout and stderr and transmit that from privileged daemon
back to the service to be displayed in HTML.
Tests:
- Unit tests and code checks pass.
- Some of the modified actions work as expected.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This is to capture stdout and stderr and transmit that from privileged daemon
back to the service to be displayed in HTML.
Tests:
- Unit tests and code checks pass.
- Some of the modified actions work as expected.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This is to capture stdout and stderr and transmit that from privileged daemon
back to the service to be displayed in HTML.
Tests:
- Unit tests and code checks pass.
- Some of the modified actions work as expected.
- systemd daemon-reload is performed during infinoted setup.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This is to capture stdout and stderr and transmit that from privileged daemon
back to the service to be displayed in HTML.
Tests:
- Unit tests and code checks pass.
- Some of the modified actions work as expected.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- When an exception is raised in subprocess.run(), for that call the stdout and
stderr are not being collected. Any previous successful calls are being
collected.
- This also fixes issues with adding an existing backup repository back after
removal. Capturing stderr is essential for raising the proper exceptions and
working correctly.
Tests:
- Remove an existing backup repository and add it back again. It fails with the
patches and succeeds with the patches.
- Remove an existing encrypted backup repository and add it back again with the
wrong password. A proper error message is shown 'Incorrect encryption
passphrase'.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- A local storage object must exist globally shared by all threads. Then
object.__dict__ is the thread specific storage. Absent this, when multiple
actions run in parallel, one will erase the thread local object of another.
Tests:
- When an error is raised in a privileged method, then the HTML error shown
contains stdout and stderr of the involved processes.
- Running functional tests on a lot of apps does not show this error anymore.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- When an error occurs during setup thread execution and the error is not due a
failed privileged action, we are left with very little information about what
went run. On the other than when a privileged action fails, we will be logging
the exception twice. But this is okay.
Tests:
- Increment the setup version of one of installed apps and raise an exception in
setup() method. Notice that exception traceback in the logged message.
- Increment the setup version of one of installed apps and raise an exception in
setup's privileged action. Notice that exception traceback in the logged
message twice.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This make it easy to find issues when looking at either main service logs or
privileged daemon logs.
Tests:
- Raise an exception in one of the privileged actions. Notice that the exception
is printed along with module name, action_name, stdout, stderr and traceback.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- When attempting to set an invalid shortcut ID or invalid user's directory as
home page, set FreedomBox UI as home page.
- Simplify the tests somewhat and avoid failure first time and skipping the test
next time.
Tests:
- Run unit tests as 'root' and 'fbx' users.
- Set home page to apache default, FreedomBox, user home page and a shortcut.
The set value is retained. The change works when visiting / with browser. The
value is as expected in Apache configuration.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
During functional tests, it was noticed that getattr() failed at the following
line. The original intent of the code is to ensure that there are no failures
when 'stdout'/'stderr' attribute are not present or when they return None.
stdout = (getattr(err, 'stdout') or b'').decode()
Tests:
- Make the UI raise incorrect password error. Notice that the error is shown
properly.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- This change means that when invalid module or action name is provided, the log
message is not printed. However, this is acceptable as those cases are rare in
production and are logged properly on the client side.
Tests:
- Run diagnostics for an app and notice that arguments are printed in privileged
daemon's journald logs.
- Remove a password from bepasty app and notice that the password argument is
not logged.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- And gracefully terminate the process after finishing the current requests
underway.
Tests:
- Trigger a long operation such as an app installation. While the operation is
underway, run 'systemctl stop freedombox-privilved.service'. Journal will show
that the SIGTERM is handled and shutdown is more or less immediately complete.
However, the whole process will wait until the ongoing request is complete and
then exit.
- During the wait period, no new requests are accepted as experienced with
'freedombox-cmd plinth is_package_manager_busy --no-args' command.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- Older privileged daemon before 25.10 did not return the stdout/stderr
properties as part of an exception. During upgrade, there is a 5 minute time
window (longer if the privileged daemon is continuously used) when privileged
daemon is the old version and the service is the newer version. During this time
any exception in the privileged task will cause this problem.
- Our goal is not to always provide backward compatibility to old version of
privileged daemon as the web interface and privileged daemon are expected to be
upgraded at the same time. However, this one is easy and is complementary to a
separate fix that addresses the core problem.
Tests:
- Perform an operation that raises an Exception in a privileged method. The
error is properly shown as an HTML message but without stdout and stderr.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
- When disconnected sshfs mounts are present, then df command prints the disk
usage for the remaining disks but prints a warning to the stderr and return a
non-zero return code. Accommodate this case and parse the information for all
the available disks.
Tests:
- Create a remote backup location and mount it. When the SSH process is killed,
it leaves a mount point that is not properly connected. View the storage page to
see that disk usage for other partitions is shown properly.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Tests:
- When arguments are not provided to freedombox-cmd it shows errors on the
console.
- When a command is successfully executed, the output is printed on the console.
- The output of the privileged daemon goes to the journald.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Due the frequency and length of execution of diagnostics, the service does not
restart when files are modified. The operation also makes other testing tasks
wait until completed. It also makes functional tests slower. So, reduce the
frequency of execution. It can always be temporarily changed when debugging
diagnostics operations is necessary.
Tests:
- Change the development interval to 18 seconds and notice that new interval is
effective in development mode but not in production mode.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Tests:
- In development mode, diagnostics task runs after about 180 seconds (with
jitter).
- In production mode, diagnostics task does not run after 180 seconds.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Closes: #2517.
Tests:
- Without the patch, run without --develop option and visit a non-exiting page
like /plinth/foo/. It results in '500 internal server error' instead of 404
non-found error.
- With the patch, the '404' page is shown. Breadcrumbs show only a link to the
home page with home icon.
- Accessing a page like /plinth/apps/bepasty/add?foo redirects it to
/plinth/apps/bepasty/add/?foo.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Tests:
- Add a custom shortcut and provide a custom icon for it in
/var/www/plinth/custom/static/icons/. Visit the page /plinth/api/1/shortcuts/.
The icon path for the custom shortcut is correct and accessible from browser.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Instead of detecting 'custom' in the name of the icons (which can happen if the
icon basename itself contains the word 'custom'), check if the icon is already
an absolute URL path.
Tests:
- Add a custom shortcut and provide a custom icon for it in
/var/www/plinth/custom/static/icons/. Visit the frontpage and ensure that the
icon is visible.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Closes: #2538
Tests:
- Without the patch, set the Zoph CLI User to "autodetect" and notice the
failure to load Zoph page.
- With the patch, set user to "autodetect" and access the app page. It is
updated to the first admin user in Zoph DB.
- Try with updating Zoph configuration.
- Try with re-running Zoph setup.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
Fixes: #2251Fixes: #2426
- Temporarily add a bug fix for the one-line style apt sources format. Upstream
submission: https://github.com/hercules-team/augeas/pull/865 .
- Currently, only two options with keys arch= or trusted= are allowed. However,
as documented in apt manual page[1], there are many different options possible.
Apt itself parses these options in a much more generic way[2][3].
- Fixes allow parsing all the different options allowed by apt. A practical
example (accepted by apt) is also provided as a test case.
Tests:
- Add the following line to one of the apt sources file in
/etc/apt/sources.list.d: "deb
[signed-by=/usr/share/keyrings/debian-archive-trixie-stable.gpg]
https://deb.debian.org/debian trixie main". In augtool, print
/augeas/files/etc/apt/sources.list.d//error.
- With the patch, run 'make build install' and errors in augtool disappear.
- In tor proxy app, enabling/disabling apt through tor works. The files in
/etc/apt/ are updated as expected.
Link: https://manpages.debian.org/trixie/apt/sources.list.5.en.html
Link: 3c9399e643/apt-pkg/sourcelist.cc (L215)
Link: 3c9399e643/apt-pkg/contrib/strutl.cc (L245)
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
