Skip to content

Commit adf5d2a

Browse files
committed
fix: fail closed on empty resolved credential in static-inject
ReadCredentialFile trims a whitespace-only secret file to "" and returns ok, and an inline mapping value may be "". The OnRequest guard only checked !ok, so an empty credential was forwarded as "Bearer " (or an empty raw header) instead of denying. Add a value == "" guard alongside !ok and lock it in with a test covering both an empty inline mapping and a whitespace-only secret file. Addresses review feedback from @huang195 on PR #655. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent 13d8062 commit adf5d2a

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

authbridge/authlib/plugins/staticinject/plugin.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,11 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi
203203
}
204204

205205
value, ok := p.resolver.Resolve(ctx, key)
206-
if !ok {
206+
// Fail closed on an empty credential as well: ReadCredentialFile trims a
207+
// whitespace-only secret file to "" and returns ok, and an inline mapping
208+
// may hold "". Without this guard either would forward an empty
209+
// "Bearer " / raw header instead of denying.
210+
if !ok || value == "" {
207211
return pipeline.DenyStatus(401, "static-inject.unresolved-key", "no credential available for the resolved key")
208212
}
209213

authbridge/authlib/plugins/staticinject/plugin_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"encoding/json"
66
"net/http"
7+
"os"
8+
"path/filepath"
79
"strings"
810
"testing"
911

@@ -241,6 +243,65 @@ func TestDenyOnUnsafeCredentialValue(t *testing.T) {
241243
}
242244
}
243245

246+
// TestDenyOnEmptyResolvedValue locks in fail-closed behavior when the resolver
247+
// returns an empty credential. ReadCredentialFile trims a whitespace-only secret
248+
// file to "" (ok=true), and an inline mapping may hold "" — both must deny rather
249+
// than forward an empty "Bearer " header.
250+
func TestDenyOnEmptyResolvedValue(t *testing.T) {
251+
t.Run("empty inline mapping value", func(t *testing.T) {
252+
p := New()
253+
cfg := `{
254+
"source": "mappings",
255+
"mappings": {"api.example.com": ""},
256+
"key_by": "host"
257+
}`
258+
if err := p.Configure(json.RawMessage(cfg)); err != nil {
259+
t.Fatalf("Configure() error = %v", err)
260+
}
261+
262+
pctx := &pipeline.Context{
263+
Host: "api.example.com",
264+
Headers: http.Header{"Authorization": []string{"Bearer PLACEHOLDER"}},
265+
}
266+
action := p.OnRequest(context.Background(), pctx)
267+
if action.Type != pipeline.Reject {
268+
t.Fatalf("OnRequest() action.Type = %v, want %v (Reject, empty credential)", action.Type, pipeline.Reject)
269+
}
270+
if got := pctx.Headers.Get("Authorization"); got != "Bearer PLACEHOLDER" {
271+
t.Errorf("Authorization header = %q, want unmodified %q", got, "Bearer PLACEHOLDER")
272+
}
273+
})
274+
275+
t.Run("whitespace-only secret file", func(t *testing.T) {
276+
dir := t.TempDir()
277+
if err := os.WriteFile(filepath.Join(dir, "api.example.com"), []byte(" \n"), 0o600); err != nil {
278+
t.Fatalf("WriteFile() error = %v", err)
279+
}
280+
281+
p := New()
282+
cfg := `{
283+
"source": "secret_dir",
284+
"secret_dir": "` + dir + `",
285+
"key_by": "host"
286+
}`
287+
if err := p.Configure(json.RawMessage(cfg)); err != nil {
288+
t.Fatalf("Configure() error = %v", err)
289+
}
290+
291+
pctx := &pipeline.Context{
292+
Host: "api.example.com",
293+
Headers: http.Header{"Authorization": []string{"Bearer PLACEHOLDER"}},
294+
}
295+
action := p.OnRequest(context.Background(), pctx)
296+
if action.Type != pipeline.Reject {
297+
t.Fatalf("OnRequest() action.Type = %v, want %v (Reject, whitespace-only file)", action.Type, pipeline.Reject)
298+
}
299+
if got := pctx.Headers.Get("Authorization"); got != "Bearer PLACEHOLDER" {
300+
t.Errorf("Authorization header = %q, want unmodified %q", got, "Bearer PLACEHOLDER")
301+
}
302+
})
303+
}
304+
244305
// =============================================================================
245306
// Name / Capabilities / ConfigSchema / Configure sanity checks
246307
// =============================================================================

0 commit comments

Comments
 (0)