nextflow-io / nextflow

A DSL for data-driven computational pipelines
http://nextflow.io
Apache License 2.0
2.71k stars 621 forks source link

`nextflow secrets` does not work with secrets that contain `$` #4977

Closed jdidion closed 4 months ago

jdidion commented 4 months ago

Bug report

A secret that contains a $ character results in an error when running a task configured to use the secret. The problem looks to be that that when the .nf-XXX.secrets file is generated, it contains export commands with double-quoted rather than single-quoted values.

Expected behavior and actual behavior

I expect that there will be no restrictions on the characters that I can store in secrets.

Steps to reproduce the problem

main.nf:

process secret_test {
  secret 'FOO'
  script:
    """
    echo '\$FOO'
    """
}

workflow {
  secret_test()
}
nextflow secrets set FOO '$bar'
nextflow run main.nf`

Program output

/dev/stdin: line 2: bar: unbound variable

The ~/.nextflow/secrets/.nf-XXX.secrets file contains:

export FOO="$bar"

Environment

pditommaso commented 4 months ago

Tricky, @marcodelapierre you may want to give it a try

marcodelapierre commented 4 months ago

Given a first look.

@pditommaso, was there a specific rationale for using double quotes in the export definitions of secrets?

At this stage, I am considering whether one of these might be a viable solution (still under investigation):

  1. use single quotes instead of double quotes in the secrets file definition
  2. allow flexibility via specific nextflow secrets set option, e.g. --single-quote or similar
  3. allow flexibility by mirroring Bash export behaviour, and hence adapt definition based on the string input by the user (does the user string contain single vs double quotes?)
pditommaso commented 4 months ago

Maybe this is the best solition

https://github.com/nextflow-io/nextflow/blob/e2e608140cdde1da39df4c911f56286015538228/modules/nextflow/src/main/groovy/nextflow/processor/TaskProcessor.groovy#L2089-L2089

dsantesmasses commented 3 months ago

Hi,

It looks like this issue was not fixed in the latest release, though it is listed as fixed https://github.com/nextflow-io/nextflow/releases/tag/v24.04.2

I tried to re-run the original example by @jdidion and got the same error using nextflow version 24.04.2

main.nf

process secret_test {
  secret 'FOO'
  script:
    """
    echo '\$FOO'
    """
}

workflow {
  secret_test()
}
$ nextflow secrets list
no secrets available
$ nextflow secrets set FOO '$bar'
$ nextflow run main.nf
 N E X T F L O W   ~  version 24.04.2

Launching `main.nf` [gloomy_faggin] DSL2 - revision: fe24afe281

executor >  local (1)
[54/b06e1d] secret_test [100%] 1 of 1, failed: 1 ✘
ERROR ~ Error executing process > 'secret_test'

Caused by:
  Process `secret_test` terminated with an error exit status (1)

Command executed:

  echo '$FOO'

Command exit status:
  1

Command output:
  (empty)

Command error:
  /dev/stdin: line 1: bar: unbound variable

Work dir:
  /data/tmp/test_secrets/work/54/b06e1df61666dcd7c71894134f188b

Tip: when you have fixed the problem you can continue the execution adding the option `-resume` to the run command line

 -- Check '.nextflow.log' file for details

Thanks!

marcodelapierre commented 3 months ago

Thank you for your report @dsantesmasses, I am looking into this

marcodelapierre commented 3 months ago

I think the behaviour you observe is caused by secrets temporary files from previous runs.

As a workaround, assuming your secrets directory is ~/.nextflow/secrets, can you try and delete those temp files via:

rm .nextflow/secrets/.nf-*.secrets

And then re-run the test case with Nextflow 24.04.0 ? I expect this to work. Please post your outcome here.

@pditommaso I see in the codebase that these secret temporary files were originally thought as being deleted at execution completion: https://github.com/nextflow-io/nextflow/blob/master/modules/nextflow/src/main/groovy/nextflow/secret/LocalSecretsProvider.groovy#L192

dsantesmasses commented 3 months ago

Thank you for the quick response @marcodelapierre.

You are right, after deleting the existing temp file ~/.nextflow/secrets/.nf-*.secrets, the error is gone.

$ nextflow secrets set FOO '$bar'
$ nextflow run main.nf

 N E X T F L O W   ~  version 24.04.2

Launching `main.nf` [insane_banach] DSL2 - revision: fe24afe281

executor >  local (1)
[a7/6396fb] secret_test [100%] 1 of 1 ✔