Skip to content

Conversation

@ppawlowski
Copy link
Contributor

@ppawlowski ppawlowski commented Jan 28, 2026

Description

This pull request changes the way how the device-agent runs inside a container. Instead of using a root user, a custom flowfuse one is created during the container build process.

Additionally, it:

  • adds a workflow that ensures the container build process is validated against every PR that introduces changes to Docker-related files
  • runs Static Application Security Test scan against the built container and pushes results to the GitHub Advanced Security
  • Adjusts the node package build pipeline to skip execution if Docker-related files are modified

Related Issue(s)

Closes #562

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@ppawlowski ppawlowski requested a review from hardillb January 28, 2026 19:02
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@ppawlowski ppawlowski marked this pull request as draft January 28, 2026 19:29
@ppawlowski ppawlowski marked this pull request as ready for review January 28, 2026 20:13
@hardillb
Copy link
Contributor

Need to think about this, mainly how it will impact things with mounted volumes as any files written will previously have been owned by root.

Will have to test

Co-authored-by: Ben Hardill <ben@flowforge.com>
Copy link
Contributor

@hardillb hardillb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My problem with this is it is potentially a breaking change, so we should merge and apply it the same time we change the default NodeJS version (to 20) at the next major release (4.0.0?).

I'm pretty sure this will break users that are extending the flowfuse/device-agent container (e.g. "windows") as the active user will change from root to a user that can not install apk packages or create new directories.

There is a workaround for the existing container which is to pass -u 1000:1000 to set the uid and gid to 1000 so as not to run as root.

Need to think about this some more

@ppawlowski
Copy link
Contributor Author

NodeJS v20 has been in maintenance for a long time, and EOL is scheduled for April/May this year. IMO if we are thinking about a breaking change release, we should aim for at least NodeJS v22 (v24 preferably) to avoid having another breaking change release in the middle of 2026.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker container should run on non-root user

3 participants