Re: [meta-virtualization][PATCH 1/7] classes: add container-nonroot-user.bbclass

On 2026-06-12 09:54:17-07:00, Bruce Ashfield wrote:
> Hi Tim,
> 
> A few things on this one — none of them blocking the intent, but worth a
> small follow-up either as a fixup in this series or a v2.
> 
> On Fri, May 29, 2026 at 18:31 -0700, Tim Orling wrote:
> 
> > For secure and production environments, we want to run containers as a
> > non-root user. Some applications, such as Python, require a $HOME
> > directory with proper permissions. Because OCI_LAYERS :directories:
> > copies with 'cp -a --no-preserve=ownership', we need a fixup function
> > to create the proper permissions and ownership in a new raw layer.
> >
> > The behavior here is inspired by dhi.io/python:3 (Docker Hardened Image)
> >
> > Signed-off-by: Tim Orling <[email protected]>
> 
> The non-root + DHI-style image is a real gap in what we ship today;
> glad to see it filled.
> 
> > +inherit extrausers
> > +
> > +NONROOT_USER ?= "nonroot"
> > +NONROOT_UID ?= "65532"
> > +NONROOT_GID ?= "65532"
> > +
> > +# ---------------------------------------------------------------------------
> > +# Create the unprivileged "nonroot" user (uid 65532, group 65532)
> > +# ---------------------------------------------------------------------------
> > +EXTRA_USERS_PARAMS = "\
> > +    groupadd -g ${NONROOT_GID} ${NONROOT_USER}; \
> > +    useradd -m -u ${NONROOT_UID} -g ${NONROOT_GID} -d /home/${NONROOT_USER}  ${NONROOT_USER}; \
> > +"
> 
> This hard-assigns EXTRA_USERS_PARAMS rather than appending. Any image
> that inherits container-nonroot-user AND wants its own extrausers (e.g.
> a recipe that needs an additional service account) loses whatever else
> was set, and the order of inheritance silently determines who wins.
> 
> Suggest:
> 
>   EXTRA_USERS_PARAMS += " \
>       groupadd -g ${NONROOT_GID} ${NONROOT_USER}; \
>       useradd -m -u ${NONROOT_UID} -g ${NONROOT_GID} \
>               -d /home/${NONROOT_USER} ${NONROOT_USER}; \
>   "
> 
> That way the class composes cleanly with anyone else who sets
> EXTRA_USERS_PARAMS earlier.
> 

Good catch and suggestion. Applied in v2.

> > +# Make sure we can write to e.g. /home/nonroot/.python_history
> > +# using :directories: in OCI_LAYERS does not preserve permissions
> > +fakeroot fix_oci_home_perms() {
> > +    cd ${IMGDEPLOYDIR}
> > +    image_name="${IMAGE_NAME}${IMAGE_NAME_SUFFIX}-oci"
> > +    layer_tar="${WORKDIR}/oci-home-fix-layer.tar"
> > +
> > +    rm -f "$layer_tar"
> > +
> > +    python3 - "$layer_tar" <<'PYEOF'
> > +import sys, tarfile, time
> > +
> > +layer_tar = sys.argv[1]
> > +mtime = int(time.time())
> > +
> > +# (path, mode, uid, gid)
> > +entries = [
> > +    ("home",         0o755, 0,     0),
> > +    ("home/${NONROOT_USER}", 0o700, ${NONROOT_UID}, ${NONROOT_GID}),
> > +]
> 
> Two things about the Python heredoc:
> 
> 1. The 'PYEOF' delimiter uses single quotes, which would normally suppress
>    shell expansion inside the heredoc — but bitbake expands ${NONROOT_USER}
>    etc. at parse time *before* the shell ever sees the body, so the
>    expansion does happen, just via a different path than the reader expects.
>    Worth a one-line comment so the next person doesn't try to "fix" it.
> 
> 2. While the values are simple here (alphanumeric user, integer uid/gid)
>    it's fine, but if someone ever sets NONROOT_USER to something with a
>    quote or backslash, the Python source is no longer well-formed. Since
>    this is a meta-virt-internal class, low risk — but a comment
>    ("NONROOT_USER must be a bare identifier") would protect it.
> 

Added comments for 1. and 2. in v2.

> > +    umoci raw add-layer --image "$image_name:${OCI_IMAGE_TAG}" "$layer_tar"
> > +    rm -f "$layer_tar"
> > +
> > +    rm -f "$image_name.tar" "$image_name-dir.tar"
> > +    ( cd "$image_name" && tar -cf "../$image_name.tar" "." )
> > +    tar -cf "$image_name-dir.tar" "$image_name"
> > +}
> > +do_image_oci[postfuncs] += "fix_oci_home_perms"
> 
> The repackaging step at the bottom assumes do_image_oci's output layout
> (image_name.tar / image_name-dir.tar) is stable. If image-oci.bbclass
> ever changes its packaging naming, this silently produces a broken
> tarball — there's no error checking against the assumption.
> 
> A defensive option: stat the expected files before rm -f / tar -cf, and
> bbwarn if either is missing rather than recreating from a possibly-empty
> directory. Or, if there's a helper in image-oci.bbclass that already
> does the repackaging, call that instead of rebuilding by hand.
> 

There is no helper in image-oci.bbclass, the repackaging is handled in
do_image_oci(). Added logic gated on OCI_IMAGE_TAR_OUTPUT to detect the
presence of $image_name/index.json and throw bbfatal if it is not found.
We could bbwarn, but I think we should fail loudly.

> Otherwise the series looks good — I'll keep going through 2/7..7/7 and
> respond per-patch as I have specific notes.
> 
> Bruce