canonical / charmcraft

Collaborate, build and publish charmed operators for Kubernetes, Linux and Windows.
Apache License 2.0
65 stars 69 forks source link

Charm plugin can include unexpected files #1900

Open lengau opened 5 days ago

lengau commented 5 days ago

Bug Description

Currently the charm plugin includes everything from its current directory, whereas it previously (2.x) only included limited items.

Changing this behaviour could be complex and breaking, because a user may accidentally depend on this behaviour to include certain files in their charm. Some ideas for minimal impact:

  1. The tests directory from the template is almost certainly not included intentionally - this is probably fine to break if someone is depending on it. Likewise for tox.ini
  2. The same for hidden files and directories under the root directory. (Hidden files levels down should be kept as-is). This would remove directories such as .git, .tox, etc.
  3. We could warn about unexpected files in the charm. At an initial glance, this would be anything in the root directory of the charm part's output except:
    • actions.yaml, charmcraft.yaml, config.yaml, manifest.yaml, metadata.yaml
    • requirements.txt
    • src/
    • lib/
    • venv/
    • Files like CONTRIBUTING, LICENSE, README, etc.
    • hooks/
    • dispatch

The previous filter: https://github.com/canonical/charmcraft/blob/hotfix/2.7/charmcraft/package.py#L227-L263

To Reproduce

Add a THIS_SHOULD_NO_BE_INCLUDED file to an existing charm that uses the charm plugin

lengau commented 5 days ago

cc @carlcsaposs-canonical @Batalex, as this conversation started here: https://matrix.to/#/!LhFxJIPEcCacgdMghH:ubuntu.com/$sPTD_ZjpvMgaTLORFj_RKqEiSyHOenZKeiM0TxK3_TQ

Batalex commented 5 days ago

Just to clarify what's actually pulled into the packed charm, hidden files at the working directory level seem to be ignored, but not those in subfolders. Symlinks are not included either. This means that sensitive files can be included in the archive, for instance my .envrc in a git worktree. It is an unlikely scenario for any charm packed and released in CI, but I wonder if that still qualifies this as a potential security issue.