Repository: caddyserver/caddy (language: go)

Issue:
Support loading placeholder content from files (for systemd credentials or docker secrets)
It would be nice to support loading secrets (e.g. api tokens for DNS auth) from files (as opposed to env vars).

E.g. using a syntax like `{file./foo/bar/baz.txt}`, which would be replaced with the contents of the file at `/foo/bar/baz.txt`.

That would support using [systemd credentials](https://systemd.io/CREDENTIALS/) or [docker secrets](https://docs.docker.com/engine/swarm/secrets/) to provide those secrets.


Discussion / hints:
Interesting idea.

One concern I have is that this placeholder would not be "instantaneous" since it would need to load from a file each time the value is invoked, so it could have negative performance consequences from constantly opening and closing files to read from them at runtime.

To clarify, placeholders are replaced at runtime whenever the config value is read, not just once when the config is loaded. The Caddyfile has support for replacing environment variables _once_ at config adapt time, but that's a special case just for the Caddyfile.

What we _could_ do is cache the value read from these files in memory for the duration the config is active. So the first time the placeholder is replaced, it would load the file, and any subsequent time it would hit the in-memory cache. A config reload would reset the cache. We'd obviously have to clearly document that the values from these file placeholders are not read live.
Good to know that `env` placeholders are only replaced at config adapt time. That was not immediately obvious to me from https://caddyserver.com/docs/conventions#placeholders.

https://github.com/caddy-dns/cloudflare#config-examples shows usage of `{env.CF_API_TOKEN}` in the Caddy JSON (i.e. not Caddyfile). Would that get replaced?
Would it get replaced, if I change the confinguration at runtime with the `/config` api endpoint and do something like
```
curl -X POST -H "Content-Type: application/json" localhost:2019/config/apps/tls/automation/policies/0/issuers -d '{
	"module": "acme",
	"challenges": {
		"dns": {
			"provider": {
				"name": "cloudflare",
				"api_token": "{env.CF_API_TOKEN}"
			}
		}
	}
}'
```
Ok, looks like it gets replaced in both cases, using `config.json` and changing the config with the http api.

I think a similar behavior would make sense for the `{file.*}` placeholder, which is what you already suggested in your response, if I'm not mistaken.
There is `{env.ENV_VAR}` and there is `{$ENV_VAR}`.
The latter is specific to the Caddyfile, will get replaced once at adapt time, and is what @francislavoie was referring to above.

See https://caddyserver.com/docs/caddyfile/concepts#environment-variables

I am sure you are aware of this already, but in case some future reader stumbles upon this, there is also [`EnvironmentFile=` in systemd](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#EnvironmentFile=) to read environment vars from a file.
Though that's obviously not the requested feature here. Just something I wanted to mention in case someone needs something similar that works *right now*.

---

Edit: I slipped on crtl+enter again and sent this comment too early :/
Actually, another detail I forgot last night -- you can use `caddy run` with the `--envfile` option which can pull your secrets from the file and load them for use with the `{env.*}` placeholders. That should _probably_ be sufficient for most users. Just means you need to format your secrets file such that it's a valid env file, but that should be viable, no?
The same applies to the `EnvironmentFile` option of systemd. It's viable (and what I'm using now) but a bit cumbersome. And does not necessarily compose well with other services using the same secret.
But I can understand, if you don't want to complicate the config further. 
What's cumbersome about using an env file?
I have to remember to put the variable name in the env file and match it up with the variable name used in the config. If another service uses the same secret, I have to load the secret there through the same variable (or create a separate version of the secret without the variable, but then I have to update 2 secrets when I want to rotate)
But wouldn't that be the same workflow if you had the secret in another file? You need to match up the filename with the file in the config.


I'm not really sure/convinced that this feature is what you're looking for, or that the extra complexity is worthwhile.
If I had the secret in a file I could use it with an arbitrary number of services supporting systemd-credentials or docker secrets.
But as I've said, I can understand if you deem the feature unnecessary and too complex for what is gained.
FWIW: [Same feature request was raised](https://github.com/caddy-dns/cloudflare/issues/46#issuecomment-1437987450) in the Caddy CloudFlare DNS plugin repo a while back.

- One scenario where `--envfile` does not work, is with `caddy-docker-proxy`, but as noted in the linked issue, that isn't the responsibility of Caddy to fix (_technically am equivalent global setting in a Caddyfile could resolve that, but may not be pragmatic_).
- Users wanting to use Docker Secrets may expect the feature to work as it is documented for Docker Swarm, but non-swarm deployment does not function in the same way. The secret support is just for local dev compatibility as noted by a Docker compose maintainer, it's just a bind mount of the file behind the scenes, none of the other secrets features apply (_including secure storage and ownership/permission controls_). Thus if someone can read the ENV of a container, there's a good chance they can read the file contents of the secret too.
- Main benefit then becomes avoiding risk of secrets leaking via ENV from software that captures the ENV to send / display (_eg: in client browser_) for debug / troubleshooting (_IIRC Django was cited as doing this_), often this happens without awareness of the software performing it (_third-party packages / modules could likewise be compromised, or performing some sort of telemetry or bug report functionality, where ENV is captured_).
- `--envfile` should avoid that concern by restricting the scope to Caddy (_and any third-party modules, but I assume they can read any secrets in config regardless_).

---

> One concern I have is that this placeholder would not be "instantaneous" since it would need to load from a file each time the value is invoked, so it could have negative performance consequences from constantly opening and closing files to read from them at runtime.

> What we _could_ do is cache the value read from these files in memory for the duration the config is active.

AFAIK (at least with Linux) that shouldn't be necessary. Files would be read from disk on initial access, but should generally exist in disk cache in memory.

So long as there is spare memory it's often used for that until a process needs the extra memory to allocate or cache is flushed (can be invoked explicitly). Once the file is read from disk again, it should likewise reside in cache.

I suppose if you have Caddy allocate memory to cache it you at least guarantee under memory constrained scenarios that it's always cached in memory, but otherwise may be redundant.

Ok; I'm not sure I know enough about Docker to make the decision on this one. It'd probably be best if I leave it up to other maintainers/contributors who are familiar/experienced with Docker on this one. :blush: 
> Ok; I'm not sure I know enough about Docker to make the decision on this one.

It's not exactly Docker specific.

I don't have links to articles atm that I could direct you to, but you will find secrets management advice discouraging the practice of configuring / passing secrets to software via ENV when using alternative sources like files is viable.

The issue regarding Docker is there is some misinformation about a feature called Docker Secrets, originally only available for a distributed deployment mode known as "Docker Swarm". It provided encrypted storage of secrets at rest and in transit to swarm nodes over the network, the secrets would then be decrypted into an in-memory tmpfs location of the container under `/run` available as a file. You could also explicitly configure the ownership/permissions of the file so that it was not world readable for example.

A "feature" added support for using the configuration syntax in a local non-swarm development environment. But this didn't use any of the features described above, it only supported sourcing a physical file on disk (_not from the encrypted vault at runtime_) and making that available as a file in the container (_without respecting any configuration to change ownership / permissions_). The benefit was less secure / useful, the user needed to be aware of this (_often they're not as there is no disclaimer and some articles / community parroting misinformation_).

It can be a beneficial practice when done properly.

---

Perhaps it's something a Caddy plugin could enable?

As it's not specific to Docker, perhaps it's better to just leave the issue open for a while to see if any other users of Caddy chime in about the secrets sourced from files as a security benefit over ENV?

I think the main concern is with the risk of ENV leaking accidentally to third-parties, but `--envfile` at least reduces the scope of that.

> Perhaps it's something a Caddy plugin could enable?

It could, but only in certain contexts like HTTP routes. It wouldn't work in other contexts like TLS issuance.

We don't have any way for a plugin to hook into global replacer. In fact there is no global replacer really, there's a `globalDefaultReplacements` function which handles placeholders which require no config to work (e.g. system and time placeholders), and this is registered by default to every replacer instanciated.

> One scenario where `--envfile` does not work, is with `caddy-docker-proxy`

It should be trivial to add that as a new CLI option to CDP. Out of scope of this issue though.

Fix patch:
diff --git a/modules/caddyhttp/templates/tplcontext.go b/modules/caddyhttp/templates/tplcontext.go
index 8ba64200f2b..4c7c86e13d2 100644
--- a/modules/caddyhttp/templates/tplcontext.go
+++ b/modules/caddyhttp/templates/tplcontext.go
@@ -249,6 +249,12 @@ func (c *TemplateContext) executeTemplateInBuffer(tplName string, buf *bytes.Buf
 
 func (c TemplateContext) funcPlaceholder(name string) string {
 	repl := c.Req.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
+
+	// For safety, we don't want to allow the file placeholder in
+	// templates because it could be used to read arbitrary files
+	// if the template contents were not trusted.
+	repl = repl.WithoutFile()
+
 	value, _ := repl.GetString(name)
 	return value
 }
diff --git a/replacer.go b/replacer.go
index 2ad5b8bcbbd..e5d2913e928 100644
--- a/replacer.go
+++ b/replacer.go
@@ -16,6 +16,7 @@ package caddy
 
 import (
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -24,6 +25,8 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"go.uber.org/zap"
 )
 
 // NewReplacer returns a new Replacer.
@@ -32,9 +35,10 @@ func NewReplacer() *Replacer {
 		static:   make(map[string]any),
 		mapMutex: &sync.RWMutex{},
 	}
-	rep.providers = []ReplacerFunc{
-		globalDefaultReplacements,
-		rep.fromStatic,
+	rep.providers = []replacementProvider{
+		globalDefaultReplacementProvider{},
+		fileReplacementProvider{},
+		ReplacerFunc(rep.fromStatic),
 	}
 	return rep
 }
@@ -46,8 +50,8 @@ func NewEmptyReplacer() *Replacer {
 		static:   make(map[string]any),
 		mapMutex: &sync.RWMutex{},
 	}
-	rep.providers = []ReplacerFunc{
-		rep.fromStatic,
+	rep.providers = []replacementProvider{
+		ReplacerFunc(rep.fromStatic),
 	}
 	return rep
 }
@@ -56,10 +60,25 @@ func NewEmptyReplacer() *Replacer {
 // A default/empty Replacer is not valid;
 // use NewReplacer to make one.
 type Replacer struct {
-	providers []ReplacerFunc
+	providers []replacementProvider
+	static    map[string]any
+	mapMutex  *sync.RWMutex
+}
 
-	static   map[string]any
-	mapMutex *sync.RWMutex
+// WithoutFile returns a copy of the current Replacer
+// without support for the {file.*} placeholder, which
+// may be unsafe in some contexts.
+//
+// EXPERIMENTAL: Subject to change or removal.
+func (r *Replacer) WithoutFile() *Replacer {
+	rep := &Replacer{static: r.static}
+	for _, v := range r.providers {
+		if _, ok := v.(fileReplacementProvider); ok {
+			continue
+		}
+		rep.providers = append(rep.providers, v)
+	}
+	return rep
 }
 
 // Map adds mapFunc to the list of value providers.
@@ -79,7 +98,7 @@ func (r *Replacer) Set(variable string, value any) {
 // the value and whether the variable was known.
 func (r *Replacer) Get(variable string) (any, bool) {
 	for _, mapFunc := range r.providers {
-		if val, ok := mapFunc(variable); ok {
+		if val, ok := mapFunc.replace(variable); ok {
 			return val, true
 		}
 	}
@@ -298,14 +317,52 @@ func ToString(val any) string {
 	}
 }
 
-// ReplacerFunc is a function that returns a replacement
-// for the given key along with true if the function is able
-// to service that key (even if the value is blank). If the
-// function does not recognize the key, false should be
-// returned.
+// ReplacerFunc is a function that returns a replacement for the
+// given key along with true if the function is able to service
+// that key (even if the value is blank). If the function does
+// not recognize the key, false should be returned.
 type ReplacerFunc func(key string) (any, bool)
 
-func globalDefaultReplacements(key string) (any, bool) {
+func (f ReplacerFunc) replace(key string) (any, bool) {
+	return f(key)
+}
+
+// replacementProvider is a type that can provide replacements
+// for placeholders. Allows for type assertion to determine
+// which type of provider it is.
+type replacementProvider interface {
+	replace(key string) (any, bool)
+}
+
+// fileReplacementsProvider handles {file.*} replacements,
+// reading a file from disk and replacing with its contents.
+type fileReplacementProvider struct{}
+
+func (f fileReplacementProvider) replace(key string) (any, bool) {
+	if !strings.HasPrefix(key, filePrefix) {
+		return nil, false
+	}
+
+	filename := key[len(filePrefix):]
+	maxSize := 1024 * 1024
+	body, err := readFileIntoBuffer(filename, maxSize)
+	if err != nil {
+		wd, _ := os.Getwd()
+		Log().Error("placeholder: failed to read file",
+			zap.String("file", filename),
+			zap.String("working_dir", wd),
+			zap.Error(err))
+		return nil, true
+	}
+	return string(body), true
+}
+
+// globalDefaultReplacementsProvider handles replacements
+// that can be used in any context, such as system variables,
+// time, or environment variables.
+type globalDefaultReplacementProvider struct{}
+
+func (f globalDefaultReplacementProvider) replace(key string) (any, bool) {
 	// check environment variable
 	const envPrefix = "env."
 	if strings.HasPrefix(key, envPrefix) {
@@ -347,6 +404,24 @@ func globalDefaultReplacements(key string) (any, bool) {
 	return nil, false
 }
 
+// readFileIntoBuffer reads the file at filePath into a size limited buffer.
+func readFileIntoBuffer(filename string, size int) ([]byte, error) {
+	file, err := os.Open(filename)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	buffer := make([]byte, size)
+	n, err := file.Read(buffer)
+	if err != nil && err != io.EOF {
+		return nil, err
+	}
+
+	// slice the buffer to the actual size
+	return buffer[:n], nil
+}
+
 // ReplacementFunc is a function that is called when a
 // replacement is being performed. It receives the
 // variable (i.e. placeholder name) and the value that
@@ -363,3 +438,5 @@ var nowFunc = time.Now
 const ReplacerCtxKey CtxKey = "replacer"
 
 const phOpen, phClose, phEscape = '{', '}', '\\'
+
+const filePrefix = "file."


Test patch:
diff --git a/caddytest/integration/testdata/foo.txt b/caddytest/integration/testdata/foo.txt
new file mode 100644
index 00000000000..19102815663
--- /dev/null
+++ b/caddytest/integration/testdata/foo.txt
@@ -0,0 +1,1 @@
+foo
\ No newline at end of file
diff --git a/replacer_test.go b/replacer_test.go
index d18ec8eeae6..cf4d321b6ed 100644
--- a/replacer_test.go
+++ b/replacer_test.go
@@ -240,9 +240,9 @@ func TestReplacerSet(t *testing.T) {
 func TestReplacerReplaceKnown(t *testing.T) {
 	rep := Replacer{
 		mapMutex: &sync.RWMutex{},
-		providers: []ReplacerFunc{
+		providers: []replacementProvider{
 			// split our possible vars to two functions (to test if both functions are called)
-			func(key string) (val any, ok bool) {
+			ReplacerFunc(func(key string) (val any, ok bool) {
 				switch key {
 				case "test1":
 					return "val1", true
@@ -255,8 +255,8 @@ func TestReplacerReplaceKnown(t *testing.T) {
 				default:
 					return "NOOO", false
 				}
-			},
-			func(key string) (val any, ok bool) {
+			}),
+			ReplacerFunc(func(key string) (val any, ok bool) {
 				switch key {
 				case "1":
 					return "test-123", true
@@ -267,7 +267,7 @@ func TestReplacerReplaceKnown(t *testing.T) {
 				default:
 					return "NOOO", false
 				}
-			},
+			}),
 		},
 	}
 
@@ -372,53 +372,99 @@ func TestReplacerMap(t *testing.T) {
 }
 
 func TestReplacerNew(t *testing.T) {
-	rep := NewReplacer()
-
-	if len(rep.providers) != 2 {
-		t.Errorf("Expected providers length '%v' got length '%v'", 2, len(rep.providers))
-	} else {
-		// test if default global replacements are added  as the first provider
-		hostname, _ := os.Hostname()
-		wd, _ := os.Getwd()
-		os.Setenv("CADDY_REPLACER_TEST", "envtest")
-		defer os.Setenv("CADDY_REPLACER_TEST", "")
-
-		for _, tc := range []struct {
-			variable string
-			value    string
-		}{
-			{
-				variable: "system.hostname",
-				value:    hostname,
-			},
-			{
-				variable: "system.slash",
-				value:    string(filepath.Separator),
-			},
-			{
-				variable: "system.os",
-				value:    runtime.GOOS,
-			},
-			{
-				variable: "system.arch",
-				value:    runtime.GOARCH,
-			},
-			{
-				variable: "system.wd",
-				value:    wd,
-			},
-			{
-				variable: "env.CADDY_REPLACER_TEST",
-				value:    "envtest",
-			},
-		} {
-			if val, ok := rep.providers[0](tc.variable); ok {
-				if val != tc.value {
-					t.Errorf("Expected value '%s' for key '%s' got '%s'", tc.value, tc.variable, val)
-				}
-			} else {
-				t.Errorf("Expected key '%s' to be recognized by first provider", tc.variable)
+	repl := NewReplacer()
+
+	if len(repl.providers) != 3 {
+		t.Errorf("Expected providers length '%v' got length '%v'", 3, len(repl.providers))
+	}
+
+	// test if default global replacements are added as the first provider
+	hostname, _ := os.Hostname()
+	wd, _ := os.Getwd()
+	os.Setenv("CADDY_REPLACER_TEST", "envtest")
+	defer os.Setenv("CADDY_REPLACER_TEST", "")
+
+	for _, tc := range []struct {
+		variable string
+		value    string
+	}{
+		{
+			variable: "system.hostname",
+			value:    hostname,
+		},
+		{
+			variable: "system.slash",
+			value:    string(filepath.Separator),
+		},
+		{
+			variable: "system.os",
+			value:    runtime.GOOS,
+		},
+		{
+			variable: "system.arch",
+			value:    runtime.GOARCH,
+		},
+		{
+			variable: "system.wd",
+			value:    wd,
+		},
+		{
+			variable: "env.CADDY_REPLACER_TEST",
+			value:    "envtest",
+		},
+	} {
+		if val, ok := repl.providers[0].replace(tc.variable); ok {
+			if val != tc.value {
+				t.Errorf("Expected value '%s' for key '%s' got '%s'", tc.value, tc.variable, val)
+			}
+		} else {
+			t.Errorf("Expected key '%s' to be recognized by first provider", tc.variable)
+		}
+	}
+
+	// test if file provider is added as the second provider
+	for _, tc := range []struct {
+		variable string
+		value    string
+	}{
+		{
+			variable: "file.caddytest/integration/testdata/foo.txt",
+			value:    "foo",
+		},
+	} {
+		if val, ok := repl.providers[1].replace(tc.variable); ok {
+			if val != tc.value {
+				t.Errorf("Expected value '%s' for key '%s' got '%s'", tc.value, tc.variable, val)
+			}
+		} else {
+			t.Errorf("Expected key '%s' to be recognized by second provider", tc.variable)
+		}
+	}
+}
+
+func TestReplacerNewWithoutFile(t *testing.T) {
+	repl := NewReplacer().WithoutFile()
+
+	for _, tc := range []struct {
+		variable string
+		value    string
+		notFound bool
+	}{
+		{
+			variable: "file.caddytest/integration/testdata/foo.txt",
+			notFound: true,
+		},
+		{
+			variable: "system.os",
+			value:    runtime.GOOS,
+		},
+	} {
+		if val, ok := repl.Get(tc.variable); ok && !tc.notFound {
+			if val != tc.value {
+				t.Errorf("Expected value '%s' for key '%s' got '%s'", tc.value, tc.variable, val)
 			}
+		} else if !tc.notFound {
+			t.Errorf("Expected key '%s' to be recognized", tc.variable)
 		}
 	}
 }
@@ -464,7 +510,7 @@ func BenchmarkReplacer(b *testing.B) {
 
 func testReplacer() Replacer {
 	return Replacer{
-		providers: make([]ReplacerFunc, 0),
+		providers: make([]replacementProvider, 0),
 		static:    make(map[string]any),
 		mapMutex:  &sync.RWMutex{},
 	}