foundryvtt-docker icon indicating copy to clipboard operation
foundryvtt-docker copied to clipboard

Allow optional secrets.json during Docker build

Open afwolfe opened this issue 1 year ago โ€ข 2 comments

Allow optional secrets.json during Docker build

๐Ÿ—ฃ Description

This PR allows the Dockerfile to take advantage of build secrets, allowing the pre-installed distribution build to support both environment variables and the secrets.json in a similar fashion to the entrypoint script.

๐Ÿ’ญ Motivation and context

If a user has already created a secrets file for use with their local container, this change allows the user to have a single source of truth for their credentials and reuse the secrets file for any future updates and builds of the image without needing to explicitly pass in the FOUNDRY_USERNAME and FOUNDRY_PASSWORD build arguments.

๐Ÿงช Testing

These changes were tested by building the image locally in a few different configurations. I tested the default build path, providing the FOUNDRY_USERNAME and FOUNDRY_PASSWORD build arguments, and the secrets file.

โœ… Pre-approval checklist

  • [x] This PR has an informative and human-readable title.
  • [x] Changes are limited to a single goal - eschew scope creep!
  • [ ] ~~All future TODOs are captured in issues, which are referenced in code comments.~~
  • [ ] ~~All relevant type-of-change labels have been added.~~
  • [x] I have read the CONTRIBUTING document.
    • NOTE: While running the pre-commit hook. I had a failure due to ansible-lint v6.14.0a0 not existing, but it passed otherwise.
  • [x] These code changes follow cisagov code standards.
  • [x] All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
  • [ ] ~~Tests have been added and/or modified to cover the changes in this PR.~~
  • [x] All new and existing tests pass.

โœ… Pre-merge checklist

  • [ ] Revert dependencies to default branches.
  • [ ] Finalize version.

โœ… Post-merge checklist

  • [ ] Add a tag or create a release.

afwolfe avatar Jun 30 '24 20:06 afwolfe

Thanks for the contribution! This is a cool idea. I should have a chance to kick the tires this week, and hopefully get it merged.
Thank you again!

felddy avatar Jul 09 '24 01:07 felddy

Hello @felddy, have you had a chance to review this yet? No rush, you had just mentioned trying it out awhile back.

I have rebased the branch on top of the latest changes and updated the version references in the README.

afwolfe avatar Aug 21 '24 20:08 afwolfe

I still think this is a cool idea, and I truly appreciate the PR. I was going to merge your changes into the v13 prototype branch but ran into an issue when I tried to test it. It wasn't a bug in your implementation. That is totally solid.

This issue I ran into was passing the credentials as a JSON blob from GitHub Actions secrets. It is an anti-pattern to pass structured data as it could lead to leakage of the secrets inside.

Structured data can cause secret redaction within logs to fail, because redaction largely relies on finding an exact match for the specific secret value. For example, do not use a blob of JSON, XML, or YAML (or similar) to encapsulate a secret value, as this significantly reduces the probability the secrets will be properly redacted. Instead, create individual secrets for each sensitive value.

See:

  • https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions

I was going to build a step that would build the JSON file from the two secrets in the repository, but that too became a bit problematic as my reusable-workflows required changing and could also lead to leakage.

I ended up going with a hybrid approach. Passing the credentials as two separate Docker build secrets, and documenting in the README how to extract credentials with jq from the command line. It isn't as elegant as your solution, but I think it will be safer, easier to maintain, and not lead to too much pain for the users.

See:

  • https://github.com/felddy/foundryvtt-docker/tree/develop-prerelease?tab=readme-ov-file#image-build-with-credentials
docker build \
  --secret id=foundry_username,src=<(jq -r '.foundry_username' path/to/credentials.json) \
  --secret id=foundry_password,src=<(jq -r '.foundry_password' path/to/credentials.json) \
  --build-arg VERSION=13.332.0 \
  --tag felddy/foundryvtt:13.332.0 \
  https://github.com/felddy/foundryvtt-docker.git#develop

I suspect that you are one of only a few people that are pre-installing distributions, and that the group as a whole is pretty savvy. I hope this solutions works well enough for you.

Thank you again for the PR and making this project that much better. You'll find a shout out on the pre-release: https://github.com/felddy/foundryvtt-docker/releases/tag/v13.332.0

Cheers!

felddy avatar Nov 08 '24 22:11 felddy

Thanks for the thoughtful review and taking the time to share the knowledge on the GitHub Actions secret best practices. I agree that while not as seamless, the final implementation is more secure. Thanks again!

afwolfe avatar Nov 12 '24 01:11 afwolfe