jacobobryant / biff

A Clojure web framework for solo developers.
https://biffweb.com
MIT License
833 stars 40 forks source link

`secret.env` values are inconsistent in dev/prod when using quotes #159

Closed rads closed 1 year ago

rads commented 1 year ago

This happens because the secrets.env file is parsed without evaluating it during bb dev, which means the usual Bash syntax is ignored. In prod, the file evaluated with . secrets.env so the quotes are handled properly.

https://github.com/jacobobryant/biff/blob/aeb52b388f551bb29b52b1af3345118cc0db7ec3/tasks/src/com/biffweb/tasks.clj#L139-L151

# secrets.env

export JDBC_PASSWORD=foo&bar
# Results (EDN):
#   bb dev:    "foo&bar"
#   bb deploy: "foo"

export JDBC_PASSWORD='foo&bar'
# Results (EDN):
#   bb dev:    "'foo&bar'"
#   bb deploy: "foo&bar"
rads commented 1 year ago

One thought I had while debugging this: I'd rather use a secrets.edn file than a secrets.env file, which would eliminate this escaping problem for Biff users. If actual env variables are needed in prod, it seems simpler to generate these export statements from secrets.edn rather than going the other way around and parsing the secrets.env file in dev.

rads commented 1 year ago
jacobobryant commented 1 year ago

This would be good to fix, one way or the other. I might need to think about this for a bit. Initial thoughts off the top of my head:

(defn secrets []
  (when (fs/exists? "secrets.env")
    (->> (sh/sh "sh" "-c" ". ./secrets.env; printenv")
         :out
         str/split-lines
         (map #(vec (str/split % #"=" 2)))
         (into {}))))
LucianoLaratelli commented 1 year ago

I use aero for this.

(defn get-config [system]
  (let [profile (if (= (System/getenv "BIFF_ENV") "prod") :prod :dev)
        config (io/resource "config.edn")]
    (-> config
        (read-config {:profile profile})
        (merge system {:development/profile profile}))))

(defn secret-hack
  "Replace biff's `secret` fn with a map of the keys it looks for."
  [system]
  (->> [:biff/jwt-secret
        :biff.middleware/cookie-secret
        :postmark/api-key
        :biff.xtdb.jdbc/password]
       (select-keys system)
       (assoc system :biff/secret)))

(def components
  [get-config
   secret-hack
   ;; ...
  ]
)

Then I have a config.edn in resources:

{:biff/port 8080
 :biff/host "0.0.0.0"
 :biff/base-url #profile {:dev "http://localhost:8080"
                          :prod "https://my.url}

 :biff/beholder-enabled #profile {:dev true
                                  :prod false}

 :biff.xtdb/dir #profile{:dev "storage/xtdb"}

 :biff.xtdb/topology #profile {:dev :standalone
                               :prod :jdbc}

 :biff.xtdb.jdbc/user "postgres"
 :biff.xtdb.jdbc/password #profile {:dev "postgres"
                                    :prod #env "JDBC_PASSWORD"}
;; etc
}

I deploy using fly and it works well. Just set the secrets with fly secrets set.

jacobobryant commented 1 year ago

@LucianoLaratelli Nice 🙂. How do you set/load secrets in dev? secrets.env as provided by biff, or something else?

jacobobryant commented 1 year ago

I've pushed a commit to the dev branch that replaces com.biffweb.tasks/secrets with this implementation. I'll probably merge that to master in a few days. In the mean time you can try it out by updating your biff dependency in tasks/deps.edn to the following:

com.biffweb/tasks {:git/url "https://github.com/jacobobryant/biff" :deps/root "tasks" :sha "690532c82e09481103ae75520e78f50519f8059b"}

This will at least fix the immediate problem of e.g. export FOO="BAR" resulting in (System/getenv "FOO") => "\"BAR\""), without requiring any configuration format changes on the user's end. Whether or not Biff should switch to using a secrets.edn file and/or Aero can always be discussed separately. My current thought is that I think the current system works well enough (at least now it should with this bug fix in place!), but I'm open to having additional thoughts. Also both of these (using a secrets.edn file/using Aero) would make for nice additions to the content library if either of you wanted to put them in gist form.

LucianoLaratelli commented 1 year ago

@LucianoLaratelli Nice 🙂. How do you set/load secrets in dev? secrets.env as provided by biff, or something else?

I have them hardcoded in the config. So I generated a separate set of cookie secret, JWT secret, etc., for dev with bb generate-secrets or as appropriate for DB etc.

LucianoLaratelli commented 1 year ago

additions to the content library

Sure, sounds good! Will put a draft together, good suggestion.

rads commented 1 year ago

@jacobobryant: Works for me, thanks for the fix!

jacobobryant commented 1 year ago

This is on master now: https://github.com/jacobobryant/biff/releases/tag/v0.7.9