- Earlier PrivateTmp=no is set freedombox-privileged.service in
4140d3b4444d2fd55ac682d066fd859cb2f034b5 and the fix was not properly tested.
Similar change was needed in plinth.service and it was not done.
- Complete the fix but this time enable private tmp and join namespaces for the
two daemons.
- This will cause issues with file uploading when plinth is run from command
line (for development purposes). This will be addressed separately.
Tests:
- Apply the change and reload systemd and restart service. Don't run plinth on
command line and run it as service.
- Uploading libraries to kiwix works
- Uploading backup tarballs works.
- Uploading TiddlyWiki and Feature Wiki wiki files works.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
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>
- The privileged service will stop by itself if left idle for 5 minutes.
However, if someone is viewing a reloading page such as during manual software
update, the privileged service is never idle.
- When freedombox package is updated to a newer version, the old version of
privileged daemon could run for a long time but newer version of freedombox
service might be running by then. This would cause protocol mismatch
problems (unless backwards compatibility is provided which is unnecessarily
hard).
- Adding PartOf=.socket in .service file means that if .socket unit is stopped
or restarted, the .service unit will be stopped or restarted too. We still don't
want the dh_installsystemd script to be starting the .service unit, so this is
ideal.
Tests:
- During fresh install of freedombox package, freedombox-privilged.socket is
started but freedombox-privileged.service is not. It is started due to socket
activation (as seen in journal logs of privileged daemon).
- During removal of freedombox package, .service is stopped when .socket unit is
stopped.
- During reinstall of freedombox package, .service is restarted when .socket
unit is restarted.
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.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>
Since debhelper 13.11.6, we don't need this hack as dh_installsystemd recognizes
the files in /usr/lib/systemd/ directory in addition to /lib/systemd/.
Tests:
- After build package with gbp. Notice that postinst script has code inserted by
dh_installsystemd for starting/restarting the service.
- Install the deb package starts service. Reinstalling the package restarts the
service.
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>
