Skip to content

Commit 3697cb9

Browse files
authored
[devbox add] --allow-insecure should handle multiple, user-specified packages (#1749)
## Summary Changes 1. Modify `devbox add`'s `--allow-insecure` flag to take a `[]string` instead of `bool`. 2. Save `allow_insecure` field in the package in the `devbox.json` config 3. Set `NIXPKGS_ALLOW_INSECURE=1` in the `nix build` during `devbox add` 4. Set the packages specified in `allow_insecure` in the generated flake's `permittedInsecurePackages`. TODO: - [x] Handle existing projects with `allow_insecure: true` in their `devbox.lock` (#1754) **Downside:** If a user erroneously specifies the wrong package name or names in `--allow-insecure=<packages>`, then a `devbox add <pkg> --allow-insecure=<packages>` outside the `devbox shell` environment will quietly work. However, the user will experience an error when running `devbox shell` (or similar) next time, and need to fix the erroneous `--allow-insecure` value. - Fix: we could fix this by doing "recomputeState" when installing-with-insecure in "ensureStateIsUpToDate", but I haven't to avoid adding complexity for this unlikely scenario. I call it unlikely because the error message (see below) specifically tells the user what values to use with `--allow-insecure=<packages>` Fixes #1337 ## How was it tested? `devbox add [email protected]` gives the following error now: ``` devbox add [email protected] Info: Adding package "[email protected]" to devbox.json [1/1] python2 [1/1] python2: Fail Error: Nix: Package ‘python-2.7.18.7’ is insecure. Known vulnerabilities: Python 2.7 has reached its end of life after 2020-01-01. See https://www.python.org/doc/sunset-python-2/. To override, use `devbox add <pkg> --allow-insecure=python-2.7.18.7` ``` Doing `devbox add [email protected] --allow-insecure=python-2.7.18.1` works with the devbox.json now having: ``` + "python": { + "version": "2.7", + "allow_insecure": ["python-2.7.18.7"], + }, ``` Also for the package reported in #1337: <img width="907" alt="Screenshot 2024-01-25 at 5 43 43 PM" src="https://github.com/jetpack-io/devbox/assets/676452/8d922d43-0c1c-4e86-ae6e-12fe9bc9e82d">
1 parent 3ac7be0 commit 3697cb9

File tree

13 files changed

+252
-41
lines changed

13 files changed

+252
-41
lines changed

internal/boxcli/add.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const toSearchForPackages = "To search for packages, use the `devbox search` com
1919

2020
type addCmdFlags struct {
2121
config configFlags
22-
allowInsecure bool
22+
allowInsecure []string
2323
disablePlugin bool
2424
platforms []string
2525
excludePlatforms []string
@@ -53,8 +53,8 @@ func addCmd() *cobra.Command {
5353
}
5454

5555
flags.config.register(command)
56-
command.Flags().BoolVar(
57-
&flags.allowInsecure, "allow-insecure", false,
56+
command.Flags().StringSliceVar(
57+
&flags.allowInsecure, "allow-insecure", []string{},
5858
"allow adding packages marked as insecure.")
5959
command.Flags().BoolVar(
6060
&flags.disablePlugin, "disable-plugin", false,

internal/devbox/devopt/devboxopts.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type Credentials struct {
4343
}
4444

4545
type AddOpts struct {
46-
AllowInsecure bool
46+
AllowInsecure []string
4747
Platforms []string
4848
ExcludePlatforms []string
4949
DisablePlugin bool

internal/devbox/packages.go

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -137,24 +137,9 @@ func (d *Devbox) setPackageOptions(pkgs []string, opts devopt.AddOpts) error {
137137
d.stderr, pkg, opts.Outputs); err != nil {
138138
return err
139139
}
140-
}
141-
142-
// Resolving here ensures we allow insecure before running ensureStateIsUpToDate
143-
// which will call print-dev-env. Resolving does not save the lockfile, we
144-
// save at the end when everything has succeeded.
145-
if opts.AllowInsecure {
146-
nix.AllowInsecurePackages()
147-
for _, name := range pkgs {
148-
p, err := d.lockfile.Resolve(name)
149-
if err != nil {
150-
return err
151-
}
152-
// TODO: Now that config packages can have fields,
153-
// we should set this in the config, not the lockfile.
154-
if !p.AllowInsecure {
155-
fmt.Fprintf(d.stderr, "Allowing insecure for %s\n", name)
156-
}
157-
p.AllowInsecure = true
140+
if err := d.cfg.Packages.SetAllowInsecure(
141+
d.stderr, pkg, opts.AllowInsecure); err != nil {
142+
return err
158143
}
159144
}
160145

@@ -179,7 +164,7 @@ func (d *Devbox) printPostAddMessage(
179164
}
180165
}
181166

182-
if len(opts.Platforms) == 0 && len(opts.ExcludePlatforms) == 0 && len(opts.Outputs) == 0 && !opts.AllowInsecure {
167+
if len(opts.Platforms) == 0 && len(opts.ExcludePlatforms) == 0 && len(opts.Outputs) == 0 && len(opts.AllowInsecure) == 0 {
183168
if len(unchangedPackageNames) == 1 {
184169
ux.Finfo(d.stderr, "Package %q was already in devbox.json and was not modified\n", unchangedPackageNames[0])
185170
} else if len(unchangedPackageNames) > 1 {
@@ -444,8 +429,12 @@ func (d *Devbox) installNixPackagesToStore(ctx context.Context) error {
444429
stepMsg := fmt.Sprintf("[%d/%d] %s", stepNum, total, pkg)
445430
fmt.Fprintf(d.stderr, stepMsg+"\n")
446431

447-
// --no-link to avoid generating the result objects
448-
err = nix.Build(ctx, []string{"--no-link"}, installable)
432+
args := &nix.BuildArgs{
433+
AllowInsecure: pkg.HasAllowInsecure(),
434+
// --no-link to avoid generating the result objects
435+
Flags: []string{"--no-link"},
436+
}
437+
err = nix.Build(ctx, args, installable)
449438
if err != nil {
450439
fmt.Fprintf(d.stderr, "%s: ", stepMsg)
451440
color.New(color.FgRed).Fprintf(d.stderr, "Fail\n")
@@ -475,6 +464,10 @@ func (d *Devbox) installNixPackagesToStore(ctx context.Context) error {
475464
)
476465
}
477466

467+
if isInsecureErr, userErr := nix.IsExitErrorInsecurePackage(err, installable); isInsecureErr {
468+
return userErr
469+
}
470+
478471
return usererr.WithUserMessage(err, "error installing package %s", pkg.Raw)
479472
}
480473

internal/devconfig/ast.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,14 @@ func (c *configAST) appendOutputs(name, fieldName string, outputs []string) {
208208
c.appendStringSliceField(name, fieldName, outputs)
209209
}
210210

211+
func (c *configAST) appendAllowInsecure(name, fieldName string, whitelist []string) {
212+
if len(whitelist) == 0 {
213+
return
214+
}
215+
216+
c.appendStringSliceField(name, fieldName, whitelist)
217+
}
218+
211219
func (c *configAST) FindPkgObject(name string) *hujson.Object {
212220
pkgs := c.packagesField(true).Value.Value.(*hujson.Object)
213221
i := c.memberIndex(pkgs, name)

internal/devconfig/config_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,38 @@ func TestSetOutputsMigrateArray(t *testing.T) {
653653
}
654654
}
655655

656+
func TestSetAllowInsecure(t *testing.T) {
657+
in, want := parseConfigTxtarTest(t, `
658+
-- in --
659+
{
660+
"packages": {
661+
"python": {
662+
"version": "2.7"
663+
}
664+
}
665+
}
666+
-- want --
667+
{
668+
"packages": {
669+
"python": {
670+
"version": "2.7",
671+
"allow_insecure": ["python-2.7.18.1"]
672+
}
673+
}
674+
}`)
675+
676+
err := in.Packages.SetAllowInsecure(io.Discard, "[email protected]", []string{"python-2.7.18.1"})
677+
if err != nil {
678+
t.Error(err)
679+
}
680+
if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" {
681+
t.Errorf("wrong parsed config json (-want +got):\n%s", diff)
682+
}
683+
if diff := cmp.Diff(want, in.Bytes()); diff != "" {
684+
t.Errorf("wrong raw config hujson (-want +got):\n%s", diff)
685+
}
686+
}
687+
656688
func TestDefault(t *testing.T) {
657689
path := filepath.Join(t.TempDir())
658690
in := DefaultConfig()

internal/devconfig/packages.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,29 @@ func (pkgs *Packages) SetOutputs(writer io.Writer, versionedName string, outputs
225225
return nil
226226
}
227227

228+
func (pkgs *Packages) SetAllowInsecure(writer io.Writer, versionedName string, whitelist []string) error {
229+
name, version := parseVersionedName(versionedName)
230+
i := pkgs.index(name, version)
231+
if i == -1 {
232+
return errors.Errorf("package %s not found", versionedName)
233+
}
234+
235+
toAdd := []string{}
236+
for _, w := range whitelist {
237+
if !slices.Contains(pkgs.Collection[i].AllowInsecure, w) {
238+
toAdd = append(toAdd, w)
239+
}
240+
}
241+
242+
if len(toAdd) > 0 {
243+
pkg := &pkgs.Collection[i]
244+
pkgs.ast.appendAllowInsecure(pkg.name, "allow_insecure", toAdd)
245+
pkg.AllowInsecure = append(pkg.AllowInsecure, toAdd...)
246+
ux.Finfo(writer, "Allowed insecure %s for package %s\n", strings.Join(toAdd, ", "), versionedName)
247+
}
248+
return nil
249+
}
250+
228251
func (pkgs *Packages) index(name, version string) int {
229252
return slices.IndexFunc(pkgs.Collection, func(p Package) bool {
230253
return p.name == name && p.Version == version
@@ -246,6 +269,10 @@ type Package struct {
246269
// Outputs is the list of outputs to use for this package, assuming
247270
// it is a nix package. If empty, the default output is used.
248271
Outputs []string `json:"outputs,omitempty"`
272+
273+
// AllowInsecure is a whitelist of packages that may be marked insecure
274+
// in nixpkgs, but are allowed by the user to be installed.
275+
AllowInsecure []string `json:"allow_insecure,omitempty"`
249276
}
250277

251278
func NewVersionOnlyPackage(name, version string) Package {
@@ -276,13 +303,18 @@ func NewPackage(name string, values map[string]any) Package {
276303
if o, ok := values["outputs"]; ok {
277304
outputs = o.([]string)
278305
}
306+
var allowInsecure []string
307+
if a, ok := values["allow_insecure"]; ok {
308+
allowInsecure = a.([]string)
309+
}
279310

280311
return Package{
281312
name: name,
282313
Version: version.(string),
283314
Platforms: platforms,
284315
ExcludedPlatforms: excludedPlatforms,
285316
Outputs: outputs,
317+
AllowInsecure: allowInsecure,
286318
}
287319
}
288320

internal/devconfig/packages_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,22 @@ func TestJsonifyConfigPackages(t *testing.T) {
174174
},
175175
},
176176
},
177+
{
178+
name: "map-with-allow-insecure-nixpkgs-reference",
179+
jsonConfig: `{"packages":{"github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c#python":` +
180+
`{"version":"2.7",` +
181+
`"allow_insecure":["python-2.7.18.1"]` +
182+
`}}}`,
183+
184+
expected: Packages{
185+
Collection: []Package{
186+
NewPackage("github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c#python", map[string]any{
187+
"version": "2.7",
188+
"allow_insecure": []string{"python-2.7.18.1"},
189+
}),
190+
},
191+
},
192+
},
177193
}
178194

179195
for _, testCase := range testCases {

internal/devpkg/package.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ type Package struct {
8686
// patches any ELF binaries to use the latest version of nixpkgs#glibc.
8787
PatchGlibc bool
8888

89+
// AllowInsecure are a list of nix packages that are whitelisted to be
90+
// installed even if they are marked as insecure.
91+
AllowInsecure []string
92+
8993
// isInstallable is true if the package may be enabled on the current platform.
9094
isInstallable bool
9195

@@ -118,6 +122,7 @@ func PackagesFromConfig(config *devconfig.Config, l lock.Locker) []*Package {
118122
pkg.DisablePlugin = cfgPkg.DisablePlugin
119123
pkg.PatchGlibc = cfgPkg.PatchGlibc && nix.SystemIsLinux()
120124
pkg.Outputs = cfgPkg.Outputs
125+
pkg.AllowInsecure = cfgPkg.AllowInsecure
121126
result = append(result, pkg)
122127
}
123128
return result
@@ -132,6 +137,7 @@ func PackageFromStringWithOptions(raw string, locker lock.Locker, opts devopt.Ad
132137
pkg.DisablePlugin = opts.DisablePlugin
133138
pkg.PatchGlibc = opts.PatchGlibc
134139
pkg.Outputs = opts.Outputs
140+
pkg.AllowInsecure = opts.AllowInsecure
135141
return pkg
136142
}
137143

@@ -572,8 +578,8 @@ func (p *Package) InputAddressedPath() (string, error) {
572578
return sysInfo.StorePath, nil
573579
}
574580

575-
func (p *Package) AllowInsecure() bool {
576-
return p.lockfile.Get(p.Raw).IsAllowInsecure()
581+
func (p *Package) HasAllowInsecure() bool {
582+
return len(p.AllowInsecure) > 0
577583
}
578584

579585
// StoreName returns the last section of the store path. Example:

internal/nix/build.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,21 @@ import (
1010
"go.jetpack.io/devbox/internal/debug"
1111
)
1212

13-
func Build(ctx context.Context, flags []string, installables ...string) error {
14-
// --impure is required for allowUnfreeEnv to work.
13+
type BuildArgs struct {
14+
AllowInsecure bool
15+
Flags []string
16+
}
17+
18+
func Build(ctx context.Context, args *BuildArgs, installables ...string) error {
19+
// --impure is required for allowUnfreeEnv/allowInsecureEnv to work.
1520
cmd := commandContext(ctx, "build", "--impure")
16-
cmd.Args = append(cmd.Args, flags...)
21+
cmd.Args = append(cmd.Args, args.Flags...)
1722
cmd.Args = append(cmd.Args, installables...)
1823
cmd.Env = allowUnfreeEnv(os.Environ())
24+
if args.AllowInsecure {
25+
debug.Log("Setting Allow-insecure env-var\n")
26+
cmd.Env = allowInsecureEnv(cmd.Env)
27+
}
1928

2029
debug.Log("Running cmd: %s\n", cmd)
2130
_, err := cmd.Output()

internal/nix/nix.go

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package nix
66
import (
77
"context"
88
"encoding/json"
9+
"fmt"
910
"io/fs"
1011
"os"
1112
"os/exec"
@@ -72,7 +73,7 @@ func (*Nix) PrintDevEnv(ctx context.Context, args *PrintDevEnvArgs) (*PrintDevEn
7273
cmd.Args = append(cmd.Args, "--json")
7374
debug.Log("Running print-dev-env cmd: %s\n", cmd)
7475
data, err = cmd.Output()
75-
if insecure, insecureErr := isExitErrorInsecurePackage(err); insecure {
76+
if insecure, insecureErr := IsExitErrorInsecurePackage(err, "" /*installable*/); insecure {
7677
return nil, insecureErr
7778
} else if err != nil {
7879
return nil, errors.Wrapf(err, "Command: %s", cmd)
@@ -223,18 +224,58 @@ func ProfileBinPath(projectDir string) string {
223224
return filepath.Join(projectDir, ProfilePath, "bin")
224225
}
225226

226-
func isExitErrorInsecurePackage(err error) (bool, error) {
227+
func IsExitErrorInsecurePackage(err error, installableOrEmpty string) (bool, error) {
227228
var exitErr *exec.ExitError
228229
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
229230
if strings.Contains(string(exitErr.Stderr), "is marked as insecure") {
230-
re := regexp.MustCompile(`Package ([^ ]+)`)
231-
match := re.FindStringSubmatch(string(exitErr.Stderr))
232-
return true, usererr.New(
233-
"Package %s is insecure. \n\n"+
234-
"To override use `devbox add <pkg> --allow-insecure`",
235-
match[0],
236-
)
231+
packageRegex := regexp.MustCompile(`Package ([^ ]+)`)
232+
packageMatch := packageRegex.FindStringSubmatch(string(exitErr.Stderr))
233+
234+
knownVulnerabilities := []string{}
235+
if installableOrEmpty != "" {
236+
knownVulnerabilities = PackageKnownVulnerabilities(installableOrEmpty)
237+
}
238+
239+
insecurePackages := parseInsecurePackagesFromExitError(string(exitErr.Stderr))
240+
241+
// Construct the error message.
242+
errMessages := []string{}
243+
errMessages = append(errMessages, fmt.Sprintf("Package %s is insecure.", packageMatch[1]))
244+
if len(knownVulnerabilities) > 0 {
245+
errMessages = append(errMessages,
246+
fmt.Sprintf("Known vulnerabilities:\n%s", strings.Join(knownVulnerabilities, "\n")))
247+
}
248+
errMessages = append(errMessages,
249+
fmt.Sprintf("To override, use `devbox add <pkg> --allow-insecure=%s`", strings.Join(insecurePackages, ", ")))
250+
251+
return true, usererr.New(strings.Join(errMessages, "\n\n"))
237252
}
238253
}
239254
return false, nil
240255
}
256+
257+
func parseInsecurePackagesFromExitError(errorMsg string) []string {
258+
insecurePackages := []string{}
259+
260+
// permittedRegex is designed to match the following:
261+
// permittedInsecurePackages = [
262+
// "package-one"
263+
// "package-two"
264+
// ];
265+
permittedRegex := regexp.MustCompile(`permittedInsecurePackages\s*=\s*\[([\s\S]*?)\]`)
266+
permittedMatch := permittedRegex.FindStringSubmatch(errorMsg)
267+
if len(permittedMatch) > 1 {
268+
packagesList := permittedMatch[1]
269+
// pick out the package name strings inside the quotes
270+
packageMatches := regexp.MustCompile(`"([^"]+)"`).FindAllStringSubmatch(packagesList, -1)
271+
272+
// Extract the insecure package names from the matches
273+
for _, packageMatch := range packageMatches {
274+
if len(packageMatch) > 1 {
275+
insecurePackages = append(insecurePackages, packageMatch[1])
276+
}
277+
}
278+
}
279+
280+
return insecurePackages
281+
}

0 commit comments

Comments
 (0)