Hello, I’ve written a tool helping me keeping my histfiles free of my secrets. I’d like to hear your thoughts and suggestions about it. I’m a Haskell noob, so please be kind. https://github.com/bionade24/histcleaner

  • jaror@kbin.social
    link
    fedilink
    arrow-up
    2
    ·
    1 year ago

    The code looks very clean! Perhaps you could add version bounds in your cabal file.

    I also saw this:

      if not exists
        then do
          createDirectory configFolder ownerModes
          pure ()
        else pure ()
    
    

    Which could be written as:

      unless exists $ createDirectory configFolder ownerModes
    
    

    I also see:

            contents <- C8.readFile filepath
            let (encSalt:_:encRest) = C8.lines contents
                rest = decodeSecrets encRest
            case decodeBase64 encSalt of
              Left _ -> error "Decoding error"
              Right salt -> pure $ Vault salt rest
    
    

    Which I would rewrite for a bit more safety

            contents <- C8.readFile filepath
            case C8.lines contents of
              encSalt:_:encRest ->
                case decodeBase64 encSalt of
                  Left _ -> error "Decoding error"
                  Right salt -> pure $ Vault salt $ decodeSecrets encRest
              _ -> error "Decoding error"
    
    

    Also, note that in this case laziness might do unexpected things. The contents of the vault will only be evaluated once you actually ask for the values. You might want to use strict fields for it like so:

    data Vault =
      Vault
        { salt :: !ByteString
        , secrets :: ![ByteString]
        }
    
    

    But that’s not enough, because this will only force the first element (more precisely the first cons-cell). To truly force all values in the list of secrets you’d have to chose another data type. Unfortunately, there’s not really any really popular strict vector type. The simplest fix is probably just to do the forcing of evaluation yourself like so:

                  Right salt -> pure $! Vault salt $!! decodeSecrets encRest
    
    

    Where $!! is from Control.DeepSeq from the deepseq package.

    • bionade24@kbin.socialOP
      link
      fedilink
      arrow-up
      1
      ·
      1 year ago

      Thx a lot for your detailed feedback. I already heard about bang notation once, but forgot about it. I think I do understand how laziness works, but I can’t understand why it’d be bad if Vault gets loaded into memory the 1st time it’s needed. Does GHC split the parsing into multiple operations because salt is used a little earlier than secrets?

      • jaror@kbin.social
        link
        fedilink
        arrow-up
        2
        ·
        1 year ago

        It’s not that the vault only gets loaded into memory the 1st time it is needed, what will happen is that the secrets will be read into memory in their raw form and only decoded on-demand. So if you only access the first secret, then it will only decode the first secret and not the rest. Haskell’s laziness is very granular like that. In this case it indeed is not that big of a deal, because it probably won’t use a lot more memory, but I’d still consider it a code smell.