Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: fix new gosec reported issues #4259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: fix new gosec reported issues #4259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions cli/azd/pkg/input/console.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ type AskerConsole struct {
currentIndent *atomic.String
// consoleWidth is the width of the underlying console window. The value is updated as the window resized. Nil when
// isTerminal is false.
consoleWidth *atomic.Int32
consoleWidth *atomic.Int64
// holds the last 2 bytes written by message or messageUX. This is used to detect when there is already an empty
// line (\n\n)
last2Byte [2]byte
Expand Down Expand Up @@ -925,13 +925,13 @@ func (c *AskerConsole) Handles() ConsoleHandles {
}

// consoleWidth the number of columns in the active console window
func consoleWidth() int {
func consoleWidth() int64 {
width, _ := consolesize.GetConsoleSize()
Copy link
Contributor Author

@weikanglim weikanglim Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just threw more memory (and maybe slightly more CPU cycles) at the problem here -- there wasn't an actual overflow risk.

The underlying row/column counts are actually uint16, being returned as int. But the safest thing for us to do is to always give more buffer.

return width
return int64(width)
}

func (c *AskerConsole) handleResize(width int) {
c.consoleWidth.Store(int32(width))
func (c *AskerConsole) handleResize(width int64) {
c.consoleWidth.Store(width)

c.spinnerLineMu.Lock()
if c.spinner.Status() == yacspin.SpinnerRunning {
Expand Down Expand Up @@ -1045,7 +1045,7 @@ func NewConsole(
c.spinner, _ = yacspin.New(spinnerConfig)
c.spinnerTerminalMode = spinnerConfig.TerminalMode
if isTerminal {
c.consoleWidth = atomic.NewInt32(int32(consoleWidth()))
c.consoleWidth = atomic.NewInt64(consoleWidth())
watchTerminalResize(c)
watchTerminalInterrupt(c)
}
Expand Down
5 changes: 2 additions & 3 deletions cli/azd/pkg/ioc/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"fmt"
"reflect"
"regexp"
"unsafe"

"github.com/golobby/container/v3"
)
Expand Down Expand Up @@ -90,11 +89,11 @@ func NewRegistrationsOnly(from *NestedContainer) *NestedContainer {
}

func getUnexportedField(field reflect.Value) interface{} {
return reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Interface()
return reflect.NewAt(field.Type(), field.Addr().UnsafePointer()).Elem().Interface()
}

func setUnexportedField(field reflect.Value, value interface{}) {
reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).
reflect.NewAt(field.Type(), field.Addr().UnsafePointer()).
Elem().
Set(reflect.ValueOf(value))
}
Expand Down
3 changes: 2 additions & 1 deletion cli/azd/pkg/output/ux/list_as_text.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ import (
func ListAsText(items []string) string {
count := len(items)
if count < 1 {
log.Panic("calling itemsCountAsText() with empty list.")
log.Panic("calling ListAsText() with empty list.")
}

if count == 1 {
return items[0]
}

if count == 2 {
//nolint:gosec // G602: slice index out of range - false positive, we know the slice has at least 2 elements
Copy link
Contributor Author

@weikanglim weikanglim Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found a better way to rewrite the function in a cleaner fashion. Here, gosec isn't correctly inferring count refers to len(items) in this usage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if a switch on len(items) would help the analysis here? 🤷.

return fmt.Sprintf("%s and %s", items[0], items[1])
}

Expand Down
2 changes: 1 addition & 1 deletion cli/azd/pkg/tools/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func extractFromTar(src, dst string) (string, error) {
// cspell: disable-next-line `Typeflag` is comming fron *tar.Header
if fileHeader.Typeflag == tar.TypeReg && fileName == "gh" {
filePath := filepath.Join(dst, fileName)
ghCliFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(fileHeader.Mode))
ghCliFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileHeader.FileInfo().Mode())
Copy link
Contributor Author

@weikanglim weikanglim Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is shifting the problem to the std library without actually changing behavior.. Under the covers, the same conversion happens. It at least safer in some regards since we're no longer holding the burden here, but I'm really open to any suggestions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't think of anything better here either.

if err != nil {
return extractedAt, err
}
Expand Down
2 changes: 1 addition & 1 deletion cli/azd/pkg/tools/pack/pack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func extractFromTar(
// cspell: disable-next-line `Typeflag` is comming fron *tar.Header
if fileHeader.Typeflag == tar.TypeReg && fileName == "pack" {
filePath := filepath.Join(out, fileName)
packCliFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(fileHeader.Mode))
packCliFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileHeader.FileInfo().Mode())
if err != nil {
return extractedAt, err
}
Expand Down
Loading