Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 62 additions & 11 deletions pkg/cli/bicep/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ type Impl struct {
func (i *Impl) PrepareTemplate(ctx context.Context, filePath string) (map[string]any, error) {
// A remote URL is downloaded to a temporary local file so it can be read or compiled like a
// local template. This mirrors the behavior users expect from tools such as kubectl.
originalPath := filePath
// displayPath is the only form of the template argument that may be shown to the user, so a
// credential embedded in a remote URL is never written to the terminal or to CI logs.
displayPath := RedactTemplatePath(filePath)
remote := isRemoteURL(filePath)
if remote {
localPath, cleanup, err := i.downloadTemplate(ctx, filePath)
Expand All @@ -92,11 +94,11 @@ func (i *Impl) PrepareTemplate(ctx context.Context, filePath string) (map[string
if strings.EqualFold(path.Ext(filePath), ".json") {
template, err := ReadARMJSON(filePath)
if err != nil && remote {
return nil, fmt.Errorf("failed to read remote template %q: %w", originalPath, err)
return nil, fmt.Errorf("failed to read remote template %q: %w", displayPath, err)
}
return template, err
} else if !strings.EqualFold(path.Ext(filePath), ".bicep") {
return nil, fmt.Errorf("the provided file %q must be a .json or .bicep file", originalPath)
return nil, fmt.Errorf("the provided file %q must be a .json or .bicep file", displayPath)
}

ok, err := IsBicepInstalled()
Expand All @@ -118,14 +120,14 @@ func (i *Impl) PrepareTemplate(ctx context.Context, filePath string) (map[string
return nil, fmt.Errorf("could not find file: %w", err)
}

step := i.Output.BeginStep("Building %s...", originalPath)
step := i.Output.BeginStep("Building %s...", displayPath)
bytes, err := i.Call("build", "--stdout", filePath)
if err != nil {
i.Output.CompleteStep(step)
if remote {
// The bicep compiler prints detailed diagnostics to stderr, so keep the wrapper
// error focused on identifying the remote source rather than guessing the cause.
return nil, fmt.Errorf("failed to build remote template %q: %w", originalPath, err)
return nil, fmt.Errorf("failed to build remote template %q: %w", displayPath, err)
}
return nil, fmt.Errorf("failed to build template: %w", err)
}
Expand All @@ -149,6 +151,18 @@ func isRemoteURL(filePath string) bool {
return strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://")
}

// RedactTemplatePath returns a display-safe form of a `rad` template argument. A local file path is
// returned unchanged; an http(s) URL has its userinfo and query-parameter values redacted so that
// credentials such as SAS tokens are never written to the terminal, to CI logs, or into generated
// output. Callers that display the template argument supplied by the user should format this value
// rather than the raw argument.
func RedactTemplatePath(templatePath string) string {
if !isRemoteURL(templatePath) {
return templatePath
}
return redactURL(templatePath)
}

// redactURL returns a display-safe copy of a URL with any userinfo and query-parameter values
// removed, so credentials embedded in the URL (basic-auth userinfo or signed query parameters such
// as SAS tokens) are never written to logs or error messages.
Expand All @@ -167,11 +181,44 @@ func redactURL(raw string) string {
}
parsed.RawQuery = query.Encode()
}
// A fragment is never needed to fetch a template, so drop it rather than risk displaying a
// credential someone placed there.
parsed.Fragment = ""
parsed.RawFragment = ""
return parsed.String()
}

// urlParseReason extracts the underlying reason from a url.Parse error, dropping the raw URL string
// it embeds (which may contain credentials).
// TemplateFileName returns the bare file name of a `rad` template argument, with any URL query
// string, fragment, and userinfo removed. Use it instead of filepath.Base when the result is
// written to disk or displayed, because filepath.Base of a URL retains the query string (and any
// credential in it).
func TemplateFileName(templatePath string) string {
if !isRemoteURL(templatePath) {
return filepath.Base(templatePath)
}
parsed, err := url.Parse(templatePath)
if err != nil {
return "template"
}
return urlFileName(parsed.Path)
}

// urlFileName extracts the final segment of a decoded URL path. Percent-decoding can turn %5C into
// a backslash, which path.Base does not treat as a separator but Windows does, so backslashes are
// normalized first. Otherwise a path such as /..%5C..%5Capp.bicep would yield "..\..\app.bicep",
// which filepath.Join would resolve outside the intended directory on Windows. A segment that is
// not usable as a file name falls back to a fixed name.
func urlFileName(urlPath string) string {
base := path.Base(strings.ReplaceAll(urlPath, `\`, "/"))
if base == "." || base == "/" || base == ".." {
return "template"
}
return base
}

// urlParseReason extracts the underlying reason from a *url.Error, dropping the raw URL string it
// embeds (which may contain credentials). It applies to both url.Parse failures and http.Client
// transport failures, since both return *url.Error.
func urlParseReason(err error) error {
var urlErr *url.Error
if errors.As(err, &urlErr) {
Expand Down Expand Up @@ -230,8 +277,10 @@ func (i *Impl) downloadTemplate(ctx context.Context, templateURL string) (string
_ = i.FileSystem.RemoveAll(dir)
}

// Preserve the original file name so compiler diagnostics reference a recognizable file.
localPath := filepath.Join(dir, path.Base(parsed.Path))
// Preserve the original file name so compiler diagnostics reference a recognizable file. The
// name is taken from the URL path only, so an encoded separator cannot place the file outside
// the temporary directory.
localPath := filepath.Join(dir, urlFileName(parsed.Path))
if err := i.FileSystem.WriteFile(localPath, body, 0600); err != nil {
cleanup()
return "", nil, fmt.Errorf("failed to write remote template to temporary file: %w", err)
Expand Down Expand Up @@ -299,7 +348,9 @@ func (i *Impl) attemptDownload(ctx context.Context, client *http.Client, templat
if ctx.Err() != nil {
return nil, 0, false, fmt.Errorf("failed to download template from %q: %w", display, context.Cause(ctx))
}
return nil, 0, true, fmt.Errorf("failed to download template from %q: %w", display, err)
// http.Client returns a *url.Error whose message embeds the raw request URL, so unwrap it
// to the underlying reason rather than leaking credentials alongside the redacted display.
return nil, 0, true, fmt.Errorf("failed to download template from %q: %w", display, urlParseReason(err))
}
defer resp.Body.Close()

Expand All @@ -322,7 +373,7 @@ func (i *Impl) attemptDownload(ctx context.Context, client *http.Client, templat
if ctx.Err() != nil {
return nil, 0, false, fmt.Errorf("failed to read template from %q: %w", display, context.Cause(ctx))
}
return nil, 0, true, fmt.Errorf("failed to read template from %q: %w", display, err)
return nil, 0, true, fmt.Errorf("failed to read template from %q: %w", display, urlParseReason(err))
}
if int64(len(body)) > maxRemoteTemplateSize {
return nil, 0, false, fmt.Errorf("template from %q exceeds the maximum allowed size of %d bytes", display, maxRemoteTemplateSize)
Expand Down
154 changes: 154 additions & 0 deletions pkg/cli/bicep/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,27 @@ func Test_downloadTemplate_DownloadFailure(t *testing.T) {
require.Contains(t, err.Error(), "failed to download template")
}

// An encoded backslash must not become a path separator: on Windows it would otherwise place the
// downloaded template outside the temporary directory.
func Test_downloadTemplate_EncodedBackslashStaysInTempDir(t *testing.T) {
content := []byte("resource foo 'Foo' = {}\n")
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(content)
}))
defer server.Close()

t.Chdir(t.TempDir())

i := newTestImpl()
localPath, cleanup, err := i.downloadTemplate(t.Context(), server.URL+"/..%5C..%5Capp.bicep")
require.NoError(t, err)
defer cleanup()

require.Equal(t, "app.bicep", filepath.Base(localPath))
require.NotContains(t, localPath, `\`)
require.NotContains(t, localPath, "..")
}

func Test_PrepareTemplate_RemoteJSON(t *testing.T) {
template := `{"$schema":"https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#","resources":[]}`
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -457,6 +478,139 @@ func Test_redactURL(t *testing.T) {
}
}

func Test_RedactTemplatePath(t *testing.T) {
tests := []struct {
name string
path string
expected string
notContain string
}{
{
name: "local path unchanged",
path: "./app.bicep",
expected: "./app.bicep",
},
{
name: "local path with query-like name unchanged",
path: "/tmp/app.bicep?notaquery",
expected: "/tmp/app.bicep?notaquery",
},
{
name: "remote url without credentials unchanged",
path: "https://example.com/app.bicep",
expected: "https://example.com/app.bicep",
},
{
name: "remote url with signed query redacted",
path: "https://example.com/app.bicep?sig=TOPSECRET",
expected: "https://example.com/app.bicep?sig=redacted",
notContain: "TOPSECRET",
},
{
name: "remote url with userinfo redacted",
path: "https://user:" + "TOPSECRET" + "@example.com/app.bicep",
expected: "https://redacted@example.com/app.bicep",
notContain: "TOPSECRET",
},
{
name: "remote url with fragment dropped",
path: "https://example.com/app.bicep#token=" + "TOPSECRET",
expected: "https://example.com/app.bicep",
notContain: "TOPSECRET",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := RedactTemplatePath(tt.path)
require.Equal(t, tt.expected, got)
if tt.notContain != "" {
require.NotContains(t, got, tt.notContain)
}
})
}
}

// PrepareTemplate must never surface the raw template argument, because a remote URL can carry a
// SAS token or basic-auth userinfo that would otherwise land in terminal and CI logs.
func Test_PrepareTemplate_RemoteErrorRedactsCredentials(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = io.WriteString(w, "{ not valid json")
}))
defer server.Close()

i := newTestImpl()
_, err := i.PrepareTemplate(t.Context(), server.URL+"/template.json?sig=TOPSECRET")
require.Error(t, err)
require.Contains(t, err.Error(), "failed to read remote template")
require.NotContains(t, err.Error(), "TOPSECRET")
require.Contains(t, err.Error(), "sig=redacted")
}

func Test_TemplateFileName(t *testing.T) {
tests := []struct {
name string
path string
expected string
}{
{name: "local relative path", path: "./app.bicep", expected: "app.bicep"},
{name: "local absolute path", path: "/tmp/dir/app.bicep", expected: "app.bicep"},
{name: "remote url", path: "https://example.com/dir/app.bicep", expected: "app.bicep"},
{
// filepath.Base would return "app.bicep?sig=abc.def" here, putting part of the
// credential into an on-disk file name.
name: "remote url with dotted query value drops the query",
path: "https://example.com/app.bicep?sig=abc.def",
expected: "app.bicep",
},
{name: "remote url with fragment drops the fragment", path: "https://example.com/app.bicep#token=abc", expected: "app.bicep"},
{
// %5C decodes to a backslash, which Windows treats as a path separator even though
// path.Base does not, so it must not survive into a file name.
name: "remote url with encoded backslash",
path: "https://example.com/folder%5Capp.bicep",
expected: "app.bicep",
},
{
name: "remote url with encoded backslash traversal",
path: "https://example.com/..%5C..%5Capp.bicep",
expected: "app.bicep",
},
{
name: "remote url with no file segment",
path: "https://example.com/",
expected: "template",
},
{name: "malformed url", path: "https://[::1/app.bicep", expected: "template"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.expected, TemplateFileName(tt.path))
})
}
}

// A transport failure returns a *url.Error whose message embeds the raw request URL, so the
// wrapped error must not carry the credential that the redacted display string removed.
func Test_downloadTemplate_TransportErrorRedactsCredentials(t *testing.T) {
original := retryBaseDelay
retryBaseDelay = time.Millisecond
defer func() { retryBaseDelay = original }()

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
url := server.URL + "/app.bicep?sig=TOPSECRET"
server.Close() // Close immediately so the connection is refused.

i := newTestImpl()
_, _, err := i.downloadTemplate(t.Context(), url)
require.Error(t, err)
require.NotContains(t, err.Error(), "TOPSECRET")
require.Contains(t, err.Error(), "sig=redacted")
}

func Test_newHTTPClient_RedirectPolicy(t *testing.T) {
client := newHTTPClient()
mustReq := func(rawURL string) *http.Request {
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/cmd/app/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,11 @@ func (r *Runner) runDeployed(ctx context.Context) error {
// under <source-branch>/app-graph.json; otherwise it is written to
// ./app-graph.json in the current working directory.
func (r *Runner) runModeled(ctx context.Context) error {
r.Output.LogInfo("Compiling %s", r.BicepFilePath)
displayPath := bicep.RedactTemplatePath(r.BicepFilePath)
r.Output.LogInfo("Compiling %s", displayPath)
template, err := r.Bicep.PrepareTemplate(ctx, r.BicepFilePath)
if err != nil {
return clierrors.Message("Failed to compile %q: %v", r.BicepFilePath, err)
return clierrors.Message("Failed to compile %q: %v", displayPath, err)
}

graph, err := cligraph.BuildModeledGraph(template, r.IncludeIcons)
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/cmd/app/graph/preview/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,11 @@ func (r *Runner) Run(ctx context.Context) error {
body.IncludeIcons = to.Ptr(true)
}
if r.BicepFilePath != "" {
r.Output.LogInfo("Compiling %s", r.BicepFilePath)
displayPath := bicep.RedactTemplatePath(r.BicepFilePath)
r.Output.LogInfo("Compiling %s", displayPath)
template, err := r.Bicep.PrepareTemplate(ctx, r.BicepFilePath)
if err != nil {
return clierrors.Message("Failed to compile %q: %v", r.BicepFilePath, err)
return clierrors.Message("Failed to compile %q: %v", displayPath, err)
}
// ExtractDependsOnEdges returns nil when the template has no
// eligible dependsOn edges, leaving body.DependsOnEdges nil so
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,12 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error {
return err
}

// If the destination file is not provided, use the base name of the file with a .yaml extension
// If the destination file is not provided, use the base name of the file with a .yaml extension.
// The name is derived from the template file name only, so a credential in a remote URL's query
// string is never used to build an on-disk file name.
if r.DestinationFile == "" {
r.DestinationFile = strings.TrimSuffix(filepath.Base(r.FilePath), filepath.Ext(r.FilePath)) + ".yaml"
templateFileName := bicep.TemplateFileName(r.FilePath)
r.DestinationFile = strings.TrimSuffix(templateFileName, filepath.Ext(templateFileName)) + ".yaml"
}

if filepath.Ext(r.DestinationFile) != ".yaml" && filepath.Ext(r.DestinationFile) != ".yml" {
Expand Down Expand Up @@ -175,7 +178,9 @@ func (r *Runner) Run(ctx context.Context) error {
return err
}

deploymentTemplate, err := r.generateDeploymentTemplate(filepath.Base(r.FilePath), template, r.Parameters)
// The template name is written into the generated manifest, so derive it from the template file
// name only to keep credentials in a remote URL out of the generated output.
deploymentTemplate, err := r.generateDeploymentTemplate(bicep.TemplateFileName(r.FilePath), template, r.Parameters)
if err != nil {
return err
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/cli/cmd/bicep/publish/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,12 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error {
// The Run function prepares a Bicep template, extracts the destination, publishes the template to the target, and logs
// a success message if no errors are encountered. An error is returned if any of the steps fail.
func (r *Runner) Run(ctx context.Context) error {
// Redact any credentials embedded in a remote template URL before displaying it.
displayFile := bicep.RedactTemplatePath(r.File)

template, err := r.Bicep.PrepareTemplate(ctx, r.File)
if err != nil {
return clierrors.MessageWithCause(err, "Failed to prepare Bicep file %q.", r.File)
return clierrors.MessageWithCause(err, "Failed to prepare Bicep file %q.", displayFile)
}
r.Template = template

Expand All @@ -173,13 +176,13 @@ func (r *Runner) Run(ctx context.Context) error {
digest, err := r.publish(ctx)
var httpErr *errcode.ErrorResponse
if errors.As(err, &httpErr) {
message := fmt.Sprintf("Failed to publish Bicep file %q to %q", r.File, r.Target)
message := fmt.Sprintf("Failed to publish Bicep file %q to %q", displayFile, r.Target)
return handleErrorResponse(httpErr, message)
} else if err != nil {
return clierrors.MessageWithCause(err, "Failed to publish Bicep file %q to %q", r.File, r.Target)
return clierrors.MessageWithCause(err, "Failed to publish Bicep file %q to %q", displayFile, r.Target)
}

r.Output.LogInfo("Successfully published Bicep file %q to %q", r.File, r.Target)
r.Output.LogInfo("Successfully published Bicep file %q to %q", displayFile, r.Target)
r.Output.LogInfo("To immutably pin the artifact, use the following Recipe url: %s", computeImmutableRecipeUrl(r.Target, digest.String()))

return nil
Expand Down
Loading
Loading