fix: ensure data paths are set correctly in docker, fixes #958#959
Conversation
dosubot
Bot
added
the
size:L
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces runtime environment detection ( ChangesRuntime env detection, config, CLI, and Docker image updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Main as main
participant Model as model
participant Cmd as cmdTinyauth
participant ConfigDump as configCmd
Main->>Model: DetectRuntimeEnv()
Model-->>Main: RuntimeEnv
Main->>Model: NewDefaultConfiguration(runtimeEnv)
Model-->>Main: Config
Main->>Cmd: AddCommand(configCmd(tConfig, loaders))
Cmd->>ConfigDump: execute config command
ConfigDump->>ConfigDump: json.MarshalIndent(config)
ConfigDump-->>Cmd: print JSON to stdout
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cmd/tinyauth/config_dump.go (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfig dump uses JSON while the config format is YAML.
The
Configstruct declares onlyyamltags, sojson.MarshalIndentemits Go field names (e.g.AppURL,Database) and includes fields markedyaml:"-"such asConfigFile. For a config-dump this diverges from the format users actually configure. Consider marshaling to YAML for parity with the config file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/config_dump.go` at line 18, The config dump in config_dump.go is using json.MarshalIndent in the config dump path, which produces JSON field names and can include fields that should be omitted from YAML output. Update the config dump logic to use YAML marshaling instead, using the existing Config type so the output matches the same yaml tags and exclusions as the config file format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 57-60: The image setup creates the tinyauth user and fixes /data
ownership, but the final container still runs as root. Update the Dockerfile to
switch the runtime user to tinyauth after the addgroup/adduser and chown steps,
so the container actually starts under the non-root account and the prepared
permissions take effect.
In `@Dockerfile.distroless`:
- Around line 50-51: The distroless Docker build is using mismatched data paths,
so the copy step in the final stage cannot find the source directory. Update the
Dockerfile so the builder stage and the runner stage use the same data location,
and ensure the COPY instruction references the directory created by the earlier
RUN mkdir/chown steps (the data setup around the /data paths in
Dockerfile.distroless).
- Around line 48-49: The distroless runner still starts as root because the
`tinyauth` account created in the builder stage is not available in the final
image and `Dockerfile.distroless` never switches users. Update the final runtime
stage to run as a non-root user by adding an appropriate `USER` directive there,
and make sure `tinyauth` is created or referenced in a way that exists in that
stage. Locate the fix around the `RUN addgroup tinyauth && adduser -DH tinyauth
-G tinyauth` step and the final runner stage configuration.
---
Nitpick comments:
In `@cmd/tinyauth/config_dump.go`:
- Line 18: The config dump in config_dump.go is using json.MarshalIndent in the
config dump path, which produces JSON field names and can include fields that
should be omitted from YAML output. Update the config dump logic to use YAML
marshaling instead, using the existing Config type so the output matches the
same yaml tags and exclusions as the config file format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 759dd09f-f2e5-4a42-ae80-2a32aa215356
⛔ Files ignored due to path filters (2)
gen/gen_env.gois excluded by!**/gen/**gen/gen_md.gois excluded by!**/gen/**
📒 Files selected for processing (6)
DockerfileDockerfile.distrolessMakefilecmd/tinyauth/config_dump.gocmd/tinyauth/tinyauth.gointernal/model/config.go
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/tinyauth/config.go (1)
11-26: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact secret-bearing fields from
configoutput.configCmdprints the fullmodel.Configas JSON, and that struct includes plaintext secrets (LDAP bind password, OAuth/OIDC client secrets, Tailscale auth key, app basic-auth password, etc.) with no redaction. Use a redacted DTO or customMarshalJSONbefore writing to stdout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/config.go` around lines 11 - 26, The config command currently marshals model.Config directly in configCmd, which exposes secret-bearing fields in stdout. Update configCmd to print a redacted representation instead, either by introducing a DTO that omits or masks sensitive values or by implementing custom MarshalJSON on model.Config so fields like LDAP bind password, OAuth/OIDC client secrets, Tailscale auth key, and app basic-auth password are not emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@cmd/tinyauth/config.go`:
- Around line 11-26: The config command currently marshals model.Config directly
in configCmd, which exposes secret-bearing fields in stdout. Update configCmd to
print a redacted representation instead, either by introducing a DTO that omits
or masks sensitive values or by implementing custom MarshalJSON on model.Config
so fields like LDAP bind password, OAuth/OIDC client secrets, Tailscale auth
key, and app basic-auth password are not emitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59cca6b4-5d6b-4e9d-b6a2-7812b843a248
📒 Files selected for processing (3)
Dockerfile.distrolesscmd/tinyauth/config.gocmd/tinyauth/tinyauth.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/tinyauth/tinyauth.go
- Dockerfile.distroless
2 participants
Summary by CodeRabbit
New Features
Bug Fixes