Skip to content

Commit 1adb4b2

Browse files
jongioCopilot
andcommitted
fix: address PR review feedback
- Fix swap.go indentation (extra tab on idx assignment) - Reject relative nextLink URLs in pagination (SSRF protection) - Clarify sanitizeControlChars comment - Use golang.org/x/term.IsTerminal for reliable TTY detection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent eba4ce8 commit 1adb4b2

3 files changed

Lines changed: 16 additions & 12 deletions

File tree

cli/azd/extensions/azure.appservice/internal/cmd/swap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ func runSwap(ctx context.Context, flags *swapFlags, rootFlags rootFlagsDefinitio
267267
return fmt.Errorf("selecting source slot: %w", err)
268268
}
269269

270-
idx := int(prompt.GetValue())
270+
idx := int(prompt.GetValue())
271271
if idx < 0 || idx >= len(srcChoices) {
272272
return fmt.Errorf("invalid source slot selection index: %d", idx)
273273
}

cli/azd/pkg/azdext/pagination.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,13 @@ func (p *Pager[T]) validateNextLink(nextLink string) error {
219219
return fmt.Errorf("invalid nextLink URL: %w", err)
220220
}
221221

222-
if u.Scheme != "" && u.Scheme != "https" {
222+
// Reject relative URLs (empty scheme) — they would fail at request time
223+
// with a "missing protocol scheme" error, and non-absolute URLs may be
224+
// used for path-based SSRF attacks.
225+
if u.Scheme == "" {
226+
return fmt.Errorf("nextLink must be an absolute URL with an HTTPS scheme")
227+
}
228+
if u.Scheme != "https" {
223229
return fmt.Errorf("nextLink must use HTTPS (got %q)", u.Scheme)
224230
}
225231

@@ -313,8 +319,8 @@ func redactURL(rawURL string) string {
313319
return u.String()
314320
}
315321

316-
// sanitizeControlChars replaces control characters with spaces to prevent
317-
// log-forging attacks in stored error bodies. Newlines and tabs are preserved.
322+
// sanitizeControlChars replaces control characters (except newlines and tabs)
323+
// with spaces to prevent log-forging attacks in stored error bodies.
318324
func sanitizeControlChars(s string) string {
319325
return strings.Map(func(r rune) rune {
320326
if unicode.IsControl(r) && r != '\n' && r != '\t' {

cli/azd/pkg/azdext/shell.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"os/exec"
1111
"runtime"
1212
"strings"
13+
14+
"golang.org/x/term"
1315
)
1416

1517
// ---------------------------------------------------------------------------
@@ -182,20 +184,16 @@ func ShellCommandWith(ctx context.Context, info ShellInfo, script string) (*exec
182184
// IsInteractiveTerminal reports whether the given file descriptor is connected
183185
// to an interactive terminal (TTY).
184186
//
185-
// Platform behavior:
186-
// - Unix: Uses [os.File.Stat] to check for character device mode.
187-
// - Windows: Uses [os.File.Stat] to check for character device mode.
187+
// Uses [golang.org/x/term.IsTerminal] for reliable cross-platform detection,
188+
// correctly distinguishing TTYs from non-interactive character devices
189+
// like /dev/null.
188190
//
189191
// This function is safe to call with nil (returns false).
190192
func IsInteractiveTerminal(f *os.File) bool {
191193
if f == nil {
192194
return false
193195
}
194-
fi, err := f.Stat()
195-
if err != nil {
196-
return false
197-
}
198-
return fi.Mode()&os.ModeCharDevice != 0
196+
return term.IsTerminal(int(f.Fd()))
199197
}
200198

201199
// IsStdinTerminal reports whether standard input is an interactive terminal.

0 commit comments

Comments
 (0)