Skip to content

secret file permissions - #790

Open
mpolitzer wants to merge 2 commits into
next/2.0from
feature/secret-file-permissions
Open

secret file permissions#790
mpolitzer wants to merge 2 commits into
next/2.0from
feature/secret-file-permissions

Conversation

@mpolitzer

Copy link
Copy Markdown

Secret handling draft

@mpolitzer mpolitzer self-assigned this Jul 22, 2026
@mpolitzer
mpolitzer force-pushed the feature/secret-file-permissions branch from 49fdcb0 to 3572316 Compare August 3, 2026 15:10
@mpolitzer mpolitzer changed the title secret file permissions (WIP) secret file permissions Aug 3, 2026
@mpolitzer
mpolitzer marked this pull request as ready for review August 3, 2026 20:26
@mpolitzer
mpolitzer requested review from renatomaia and vfusco August 3, 2026 20:26
@mpolitzer
mpolitzer force-pushed the feature/secret-file-permissions branch from 3572316 to 53ec293 Compare August 5, 2026 14:11
@mpolitzer
mpolitzer requested a lite review from Copilot August 7, 2026 16:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a policy-based mechanism for reading configuration values from *_FILE environment variables with permission/type validation, aiming to prevent insecure secret mounts from being silently accepted.

Changes:

  • Added SecretFilePolicy tiers and ReadConfigFileWithPolicy to validate file type/permissions before reading sensitive config files.
  • Updated generated config getters (and generator) to use the appropriate secret-file policy per variable.
  • Added documentation and Docker Compose updates to describe/enforce recommended secret file modes and ownership.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/config/secret_files.go Defines secret file policy tiers and the policy-enforcing read helper.
internal/config/secret_files_windows.go Provides a Windows no-op permission checker (regular-file check only).
internal/config/secret_files_unix.go Implements POSIX permission/ownership enforcement for secret file policies.
internal/config/secret_files_test.go Adds unit tests for policy behavior (regular file, permissions).
internal/config/secret_files_getter_test.go Adds end-to-end tests validating generated getters enforce policies.
internal/config/generated.go Switches generated getters from os.ReadFile to ReadConfigFileWithPolicy.
internal/config/generate/env.go Adds file-policy metadata to the generator schema and validates it.
internal/config/generate/docs.go Emits human-readable file policy information into generated docs.
internal/config/generate/Config.toml Annotates file-backed config entries with the intended file-policy.
internal/config/generate/code.go Generates policy-aware file reads in getters via ReadConfigFileWithPolicy.
docs/secret-files.md Documents secret-file policy tiers, rationale, and Docker Compose guidance.
compose.yaml Updates secrets to set uid/gid/mode for stricter file policy compliance.
compose.individual-services.yaml Same as above for individual-service compose configuration.
Files not reviewed (1)
  • internal/config/generated.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/config/secret_files.go Outdated
Comment thread internal/config/secret_files_unix.go Outdated
Comment thread docs/secret-files.md
@mpolitzer
mpolitzer force-pushed the feature/secret-file-permissions branch from 53ec293 to 38c26a5 Compare August 11, 2026 02:35
@mpolitzer
mpolitzer requested a balanced review from Copilot August 11, 2026 02:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • internal/config/generated.go: Generated file
Suppressed comments (5)

docs/secret-files.md:41

  • The mask 0o007 also rejects world-executable files, so this prose still understates the credential policy and contradicts the implementation/error message. Include execute access in the documented restriction.
- **must not be world-readable or world-writable**
  (`mode & 0o007` must be `0`).

docs/secret-files.md:108

  • This compatibility note is incorrect for secrets whose top-level source is file: current Docker Compose still warns that uid, gid, and mode are unsupported and ignores them. Therefore the preceding example cannot enforce 0400/uid 102 as written. Document that host metadata is retained and revise the example to use a supported content/environment source or an initialization step.
> Note: the `uid`, `gid`, and `mode` fields on service secrets require a recent
> Docker Compose version. Verify support in your environment; if they are
> unsupported, mount the secret as a bind volume with the host file owned by uid
> 102 and mode `0400` instead.

internal/config/generate/docs.go:73

  • The left-trim markers remove the newline emitted after the Type bullet (and before end), so generated file-backed entries concatenate * **File policy:** onto adjacent bullets instead of placing it on its own line. Preserve the preceding newline and trim only the newline after the opening action.
{{- if .File}}
* **File policy:** {{filePolicyDesc .FilePolicy}}
{{- end}}

compose.yaml:41

  • The PostgreSQL service does not consume CARTESI_DATABASE_CONNECTION_FILE or any corresponding POSTGRES_*_FILE setting, so this mount exposes the full DSN to a container that never reads it. Remove the unused secret mount; mounting it here does not affect ownership or permissions of the separate mounts in the node/migration services.
    secrets:
      - source: database_connection
        target: database_connection
        uid: "102"
        gid: "102"
        mode: 0440

internal/config/secret_files_unix.go:51

  • The strict policy's ownership-rejection branch is not covered by the new tests; they only exercise files owned by the test process. Add a unit test using an os.FileInfo stub whose Sys() returns a syscall.Stat_t with a different UID, and assert that strict rejects it while non-strict policies do not enforce ownership.
	if euid := syscall.Geteuid(); int(stat.Uid) != euid {
		return fmt.Errorf(
			"file %q is owned by uid %d but the current effective user is %d",
			path, stat.Uid, euid,
		)

Comment thread internal/config/secret_files_unix.go
Comment thread compose.yaml
Comment on lines +82 to +86
- source: auth_mnemonic
target: auth_mnemonic
uid: "102"
gid: "102"
mode: 0400

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, this is known issue.

Comment on lines +71 to +75
- source: auth_mnemonic
target: auth_mnemonic
uid: "102"
gid: "102"
mode: 0400

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, this is known issue.

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.

2 participants