Skip to content

Commit

Permalink
Don't prompt to allow if user explicitly denied (#1158)
Browse files Browse the repository at this point in the history
Currently if an `Allow` check fails direnv will prompt the user to
`direnv allow` the file.  However, `direnv` will do that even if the
user explicitly ran a `direnv deny` command, which is not necessarily
the desired behavior.

In particular, a user might have `direnv` installed and enabled but
might not wish to enable the `.envrc` file for every repository that
they interact with.  However, currently `direnv` will always prompt the
user to `direnv allow` every time they `cd` into a repository with a
`.envrc` file.

This fixes that by recording when the user explicitly denied a `.envrc`
file so that `direnv` knows not to prompt again.

There is one key difference between how `direnv allow` and
`direnv deny` record the hash.  `direnv allow` records a grant based
on the file's (name × content) hash whereas `direnv deny` records
a revocation based on the file's name alone.  The reason behind this
difference is so that `direnv` doesn't keep prompting the user to
`direnv allow` a file if the file changes over time (e.g. the user
pulls in new changes to the file from the repository origin).

This change also includes some small fixes to not fail or emit a
warning if a user runs `direnv allow` or `direnv deny` twice in a row.
Before those fixes `direnv` would fail with an error message like:

```
direnv: error stat …/.local/share/direnv/allow/…: no such file or directory
```

… and after this change `direnv` will silently exit (successfully) if
there is nothing to be done.
  • Loading branch information
Gabriella439 committed Oct 16, 2023
1 parent f2fbec8 commit b80f567
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 17 deletions.
5 changes: 5 additions & 0 deletions internal/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ func (config *Config) AllowDir() string {
return filepath.Join(config.DataDir, "allow")
}

// DenyDir is the folder where all the "deny" files are stored.
func (config *Config) DenyDir() string {
return filepath.Join(config.DataDir, "deny")
}

// LoadedRC returns a RC file if any has been loaded
func (config *Config) LoadedRC() *RC {
if config.Env[DIRENV_FILE] == "" {
Expand Down
114 changes: 97 additions & 17 deletions internal/cmd/rc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package cmd
import (
"context"
"crypto/sha256"
"errors"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
"os/signal"
Expand All @@ -17,6 +19,7 @@ import (
type RC struct {
path string
allowPath string
denyPath string
times FileTimes
config *Config
}
Expand All @@ -33,12 +36,19 @@ func FindRC(wd string, config *Config) (*RC, error) {

// RCFromPath inits the RC from a given path
func RCFromPath(path string, config *Config) (*RC, error) {
hash, err := fileHash(path)
fileHash, err := fileHash(path)
if err != nil {
return nil, err
}

allowPath := filepath.Join(config.AllowDir(), hash)
allowPath := filepath.Join(config.AllowDir(), fileHash)

pathHash, err := pathHash(path)
if err != nil {
return nil, err
}

denyPath := filepath.Join(config.DenyDir(), pathHash)

times := NewFileTimes()

Expand All @@ -52,24 +62,37 @@ func RCFromPath(path string, config *Config) (*RC, error) {
return nil, err
}

return &RC{path, allowPath, times, config}, nil
err = times.Update(denyPath)
if err != nil {
return nil, err
}

return &RC{path, allowPath, denyPath, times, config}, nil
}

// RCFromEnv inits the RC from the environment
func RCFromEnv(path, marshalledTimes string, config *Config) *RC {
hash, err := fileHash(path)
fileHash, err := fileHash(path)
if err != nil {
return nil
}

allowPath := filepath.Join(config.AllowDir(), hash)
allowPath := filepath.Join(config.AllowDir(), fileHash)

times := NewFileTimes()
err = times.Unmarshal(marshalledTimes)
if err != nil {
return nil
}
return &RC{path, allowPath, times, config}

pathHash, err := pathHash(path)
if err != nil {
return nil
}

denyPath := filepath.Join(config.DenyDir(), pathHash)

return &RC{path, allowPath, denyPath, times, config}
}

// Allow grants the RC as allowed to load
Expand All @@ -83,44 +106,83 @@ func (rc *RC) Allow() (err error) {
if err = allow(rc.path, rc.allowPath); err != nil {
return
}
err = rc.times.Update(rc.allowPath)
return
if err = rc.times.Update(rc.allowPath); err != nil {
return
}
if _, err = os.Stat(rc.denyPath); err != nil {
if errors.Is(err, fs.ErrNotExist) {
return nil
}
return err
}
return os.Remove(rc.denyPath)
}

// Deny revokes the permission of the RC file to load
func (rc *RC) Deny() error {
func (rc *RC) Deny() (err error) {
if err = os.MkdirAll(filepath.Dir(rc.denyPath), 0755); err != nil {
return
}

// G306: Expect WriteFile permissions to be 0600 or less
// #nosec
if err = os.WriteFile(rc.denyPath, []byte(rc.path+"\n"), 0644); err != nil {
return
}

if _, err = os.Stat(rc.allowPath); err != nil {
if errors.Is(err, fs.ErrNotExist) {
return nil
}
return err
}

return os.Remove(rc.allowPath)
}

type AllowStatus int

const (
Allowed AllowStatus = iota
NotAllowed
Denied
)

// Allowed checks if the RC file has been granted loading
func (rc *RC) Allowed() bool {
func (rc *RC) Allowed() AllowStatus {
_, err := os.Stat(rc.denyPath)

if err == nil {
return Denied
}

// happy path is if this envrc has been explicitly allowed, O(1)ish common case
_, err := os.Stat(rc.allowPath)
_, err = os.Stat(rc.allowPath)

if err == nil {
return true
return Allowed
}

// when whitelisting we want to be (path) absolutely sure we've not been duped with a symlink
path, err := filepath.Abs(rc.path)
// seems unlikely that we'd hit this, but have to handle it
if err != nil {
return false
return NotAllowed
}

// exact whitelists are O(1)ish to check, so look there first
if rc.config.WhitelistExact[path] {
return true
return Allowed
}

// finally we check if any of our whitelist prefixes match
for _, prefix := range rc.config.WhitelistPrefix {
if strings.HasPrefix(path, prefix) {
return true
return Allowed
}
}

return false
return NotAllowed
}

// Path returns the path to the RC file
Expand Down Expand Up @@ -153,9 +215,13 @@ func (rc *RC) Load(previousEnv Env) (newEnv Env, err error) {
}()

// Abort if the file is not allowed
if !rc.Allowed() {
switch rc.Allowed() {
case NotAllowed:
err = fmt.Errorf(notAllowed, rc.Path())
return
case Allowed:
case Denied:
return
}

// Allow RC loads to be canceled with SIGINT
Expand Down Expand Up @@ -289,6 +355,20 @@ func fileHash(path string) (hash string, err error) {
return fmt.Sprintf("%x", hasher.Sum(nil)), nil
}

func pathHash(path string) (hash string, err error) {
if path, err = filepath.Abs(path); err != nil {
return
}

hasher := sha256.New()
_, err = hasher.Write([]byte(path + "\n"))
if err != nil {
return
}

return fmt.Sprintf("%x", hasher.Sum(nil)), nil
}

// Creates a file

func touch(path string) (err error) {
Expand Down

0 comments on commit b80f567

Please sign in to comment.