diff --git a/docs/runnable-binding-delivery.md b/docs/runnable-binding-delivery.md index c715e5d8..f748df65 100644 --- a/docs/runnable-binding-delivery.md +++ b/docs/runnable-binding-delivery.md @@ -206,8 +206,10 @@ Everything fails closed, and as early as the information exists: ConfigMap and Secret it loads as the container's environment, and refuses a file ConfigMap or Secret above 1 MiB, the API server's object limit. - **Read time.** A file carrier whose path is relative, is not a regular file, - exceeds `resources.MaxFileCarrierBytes`, or cannot be read is an error, not - an absent value: an unreadable credential never reads as an unset one. A key + is empty, exceeds `resources.MaxFileCarrierBytes`, or cannot be read is an + error, not an absent value: an unreadable credential never reads as an unset + one. An empty file is a truncated write or a mount race, since only a value + above `resources.FileCarrierThreshold` is delivered by file. A key delivered both inline and by file is refused as two sources for one fact. - **Install time.** A contract digest that does not cover the contract delivered with it, a route that is not the operation the value names, a policy outside diff --git a/resources/file_carrier.go b/resources/file_carrier.go index eb65c7d1..8a79e575 100644 --- a/resources/file_carrier.go +++ b/resources/file_carrier.go @@ -225,9 +225,14 @@ func CheckProcessEnvironment(envs []*EnvironmentVariable) error { // ResolveFileCarriers reads every file carrier of environ ("KEY=VALUE" // strings, as os.Environ returns them) and returns the value key of each with // the content of its file. A carrier whose value key is also set inline, a -// relative path, a file that is not a regular file, is larger than +// relative path, a file that is not a regular file, is empty, is larger than // MaxFileCarrierBytes or cannot be read, and a secret file accessible by // others, are errors: an unreadable value never reads as an unset one. +// +// An empty file is refused because a value is delivered by file only when it +// is larger than FileCarrierThreshold (deliverByFile): an empty one is a +// truncated write or a mount race, never a value the host left unset, and +// reading it as absent would let a lookup step past a broken delivery. func ResolveFileCarriers(environ []string) (map[string]string, error) { inline := map[string]bool{} carriers := map[string]string{} @@ -292,5 +297,8 @@ func ReadFileCarrier(key, path string) (string, error) { if len(content) > MaxFileCarrierBytes { return "", fmt.Errorf("%w: %s: exceeds %d bytes", ErrFileCarrier, key, MaxFileCarrierBytes) } + if len(content) == 0 { + return "", fmt.Errorf("%w: %s: delivered file is empty", ErrFileCarrier, key) + } return string(content), nil } diff --git a/resources/file_carrier_test.go b/resources/file_carrier_test.go index 63d6ca52..4811d7cd 100644 --- a/resources/file_carrier_test.go +++ b/resources/file_carrier_test.go @@ -87,6 +87,8 @@ func TestResolveFileCarriersFailsClosed(t *testing.T) { dir := t.TempDir() value := filepath.Join(dir, "value") require.NoError(t, os.WriteFile(value, []byte("content"), 0o600)) + empty := filepath.Join(dir, "empty") + require.NoError(t, os.WriteFile(empty, nil, 0o600)) public := "CODEFLY__WORKSPACE_CONFIGURATION__GROUP__VALUE" secret := "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__GROUP__VALUE" @@ -99,6 +101,9 @@ func TestResolveFileCarriersFailsClosed(t *testing.T) { "relative path": {resources.FileCarrierKey(public) + "=value"}, "missing file": {resources.FileCarrierKey(public) + "=" + filepath.Join(dir, "absent")}, "a directory": {resources.FileCarrierKey(public) + "=" + dir}, + // A value goes by file only above FileCarrierThreshold, so an empty + // file is a broken delivery, not an unset value. + "an empty file": {resources.FileCarrierKey(public) + "=" + empty}, } for name, environ := range cases { _, err := resources.ResolveFileCarriers(environ)