Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use os-agnostic os.UserConfigDir func #114

Merged
merged 16 commits into from
Sep 29, 2023

Conversation

dogancanbakir
Copy link
Member

Closes #113

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggesting use of helper functions from utils regarding home,config dirs
also we should implement a migration of old config files to new location

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm !

  • Added Unit Test for migration and other minor improvements

@Mzack9999 @ehsandeep . once this PR is merged and reflected in all downstream tools like tlsx,nuclei etc. all config files from old config directory will be moved to new config directory by renaming them at once

Just in case we should also test this PR with tools like subfinder , tlsx to avoid any unforseen situations since this is breaking change and cannot be reversed

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As suggested by @tarunKoyalwar, please test the following scenarios via the replace directive:

  • Individual tool migration (e.g. tlsx with standard settings, specifying a custom directory, a directory with invalid configuration files)
  • Tools using other tools with goflags as a dependency as libraries (we want to avoid automatic messy migrations with overlapping/misleading names)

@dogancanbakir
Copy link
Member Author

Individual tool migration (e.g. tlsx with standard settings, specifying a custom directory, a directory with invalid configuration files)

Nothing happens when HomeDirOrDefault (current user HOME path) GetToolConfigDir returns the same dir. When changing HOME path to something else, for example /mynewhome/fortytwo then migration happens from /home/fortytwo/.config/<toolname> to /mynewhome/fortytwo/.config/<toolname>. When there are invalid config files, it will still move them to /mynewhome/fortytwo/.config/<toolname> but will create the necessary files, for example., config.yaml.

Tools using other tools with goflags as a dependency as libraries (we want to avoid automatic messy migrations with overlapping/misleading names)

I tested similar scenarios using subfinder as a library. The behavior is the same as using the tool from the CLI.

It would be beneficial to have someone else test these cases in order to ensure that nothing is overlooked.

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - running it with nuclei (which uses multiple tools as libraries) the only moved config files appears to be nuclei one.

Requesting review from @ehsandeep for visibility and too coordinate release since it's a breaking change

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • nested directories under ($HOME/.config/$PROJECT_NAME) are not moved to a new location
  • remove moved / empty directory.

@dogancanbakir
Copy link
Member Author

Fixed in projectdiscovery/utils#198. I will update this as soon as it is merged.

@tarunKoyalwar
Copy link
Member

Bash script to check migration

#!/bin/sh

### Create migration directory
mkdir migration_tests

export WORK_DIR=$(pwd)"/migration_tests"
export PD_GIT="https://github.com/projectdiscovery"
export SEP="\n====================================\n"
export GOFLAGS_PR="https://github.com/dogancanbakir/goflags"

### Clone goflags repo
cd $WORK_DIR && git clone $GOFLAGS_PR 

printf "\nCheckout os_agnostic_configdir branch\n"

cd $WORK_DIR/goflags && git checkout os_agnostic_configdir  && export GOFLAGS_DIR=$(pwd)

printf "\nUsing goflags from $GOFLAGS_DIR\n"

function check_migration() {
    ### Clone repo
    ### $1 is the repo name
    ### $2 is go.mod directory if at root leave it empty
    git clone $PD_GIT/$1.git
    printf $SEP
    cd $1$2 && go mod edit -replace github.com/projectdiscovery/goflags=$GOFLAGS_DIR && go mod tidy
    printf $SEP
    printf "Content of previous config directory\n"
    tree $HOME/.config/$1
    printf $SEP
    printf "Staring migration\n"
    cd $WORK_DIR/$1$2/cmd/$1 && go run . 
    printf $SEP
    printf "Content of new config directory and old config directory\n"
    tree $HOME/Library/Application\ Support/$1 $HOME/.config/$1
    printf $SEP
    rm -rf $WORK_DIR
}


# Check first argument
if [ "$1" == "uncover" ]; then
    # First part of the script
    printf "Starting uncover migration\n"
    cd $WORK_DIR && check_migration uncover ""
    # Add your commands here
elif [ "$1" == "nuclei" ]; then
    # Second part of the script
    printf "Starting nuclei migration\n"
    cd $WORK_DIR &&  check_migration nuclei "/v2"
    # Add your commands here
else
    # Error message if first argument is not "uncover" or "nuclei"
    echo "Error: Please input 'uncover' or 'nuclei' as first argument."
    exit 1
fi

@tarunKoyalwar
Copy link
Member

Migration of uncover config ✅

$ bash migration.sh uncover | tee uncover.log
Cloning into 'goflags'...
remote: Enumerating objects: 813, done.
remote: Counting objects: 100% (385/385), done.
remote: Compressing objects: 100% (171/171), done.
remote: Total 813 (delta 263), reused 262 (delta 205), pack-reused 428
Receiving objects: 100% (813/813), 6.66 MiB | 18.38 MiB/s, done.
Resolving deltas: 100% (477/477), done.

Checkout os_agnostic_configdir branch
Switched to a new branch 'os_agnostic_configdir'
branch 'os_agnostic_configdir' set up to track 'origin/os_agnostic_configdir'.

Using goflags from /Users/tarun/testing/migration_tests/goflags
Starting uncover migration
Cloning into 'uncover'...
remote: Enumerating objects: 1570, done.
remote: Counting objects: 100% (342/342), done.
remote: Compressing objects: 100% (212/212), done.
remote: Total 1570 (delta 243), reused 182 (delta 128), pack-reused 1228
Receiving objects: 100% (1570/1570), 427.06 KiB | 2.15 MiB/s, done.
Resolving deltas: 100% (1030/1030), done.

====================================
go: finding module for package github.com/kr/pretty
go: found github.com/kr/pretty in github.com/kr/pretty v0.3.1

====================================
Content of previous config directory
/Users/tarun/.config/uncover
├── config.yaml
├── provider-config.yaml
└── testDir
    └── backup.yaml

2 directories, 3 files

====================================
Staring migration

  __  ______  _________ _   _____  _____
 / / / / __ \/ ___/ __ \ | / / _ \/ ___/
/ /_/ / / / / /__/ /_/ / |/ /  __/ /    
\__,_/_/ /_/\___/\____/|___/\___/_/

		projectdiscovery.io

[INF] Current uncover version v1.0.5 (latest)
[FTL] Program exiting: no query provided
exit status 1

====================================
Content of new config directory and old config directory
/Users/tarun/Library/Application Support/uncover
├── config.yaml
├── provider-config.yaml
└── testDir
    └── backup.yaml
/Users/tarun/.config/uncover  [error opening dir]

2 directories, 3 files

@tarunKoyalwar
Copy link
Member

Migration of nuclei config ✅

$  bash migration.sh nuclei | tee nuclei2.log
Cloning into 'goflags'...
remote: Enumerating objects: 813, done.
remote: Counting objects: 100% (371/371), done.
remote: Compressing objects: 100% (167/167), done.
remote: Total 813 (delta 249), reused 251 (delta 195), pack-reused 442
Receiving objects: 100% (813/813), 6.66 MiB | 963.00 KiB/s, done.
Resolving deltas: 100% (477/477), done.

Checkout os_agnostic_configdir branch
Switched to a new branch 'os_agnostic_configdir'
branch 'os_agnostic_configdir' set up to track 'origin/os_agnostic_configdir'.

Using goflags from /Users/tarun/testing/migration_tests/goflags
Starting nuclei migration
Cloning into 'nuclei'...
remote: Enumerating objects: 34429, done.
remote: Counting objects: 100% (562/562), done.
remote: Compressing objects: 100% (257/257), done.
remote: Total 34429 (delta 331), reused 458 (delta 291), pack-reused 33867
Receiving objects: 100% (34429/34429), 19.97 MiB | 16.49 MiB/s, done.
Resolving deltas: 100% (24016/24016), done.

====================================

====================================
Content of previous config directory
/Users/tarun/.config/nuclei
├── config.yaml
├── reporting-config.yaml
├── resume-cip9isbjtoj1gtd04odg.cfg
├── resume-cip9j0rjtoj1h2044oh0.cfg
├── resume-cipe5qjjtojec3ta4kmg.cfg
├── resume-ciqp8tbjtoj157c79s70.cfg
├── resume-ciqplu3jtoj6nag115ug.cfg
├── resume-cirskijjtoj94ibj1gtg.cfg
├── resume-cirvs7bjtoj5mbth5p00.cfg
├── resume-cis0693jtoj01ibq11h0.cfg
├── resume-cis0k8jjtoj49udf3v20.cfg
├── resume-cis0kdrjtoj4a0df7kbg.cfg
├── resume-cis0kfjjtoj4a21ooljg.cfg
├── resume-cis0kmbjtoj4a4m92qa0.cfg
├── resume-cisir3rjtoj2j6hvnimg.cfg
├── resume-cisirn3jtoj2k3d2lg00.cfg
└── resume-cit52sjjtoj0p5rhtlr0.cfg

1 directory, 17 files

====================================
Staring migration

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v2.9.9

		projectdiscovery.io

[ERR] Could not read nuclei-ignore file: open /Users/tarun/.config/nuclei/.nuclei-ignore: no such file or directory
[INF] Current nuclei version: v2.9.9 (latest)
[INF] Current nuclei-templates version: v9.5.8 (latest)
[INF] New templates added in latest release: 113
[INF] Templates loaded for current scan: 6466
[INF] No results found. Better luck next time!

====================================
Content of new config directory and old config directory
/Users/tarun/Library/Application Support/nuclei
├── config.yaml
├── reporting-config.yaml
├── resume-cip9isbjtoj1gtd04odg.cfg
├── resume-cip9j0rjtoj1h2044oh0.cfg
├── resume-cipe5qjjtojec3ta4kmg.cfg
├── resume-ciqp8tbjtoj157c79s70.cfg
├── resume-ciqplu3jtoj6nag115ug.cfg
├── resume-cirskijjtoj94ibj1gtg.cfg
├── resume-cirvs7bjtoj5mbth5p00.cfg
├── resume-cis0693jtoj01ibq11h0.cfg
├── resume-cis0k8jjtoj49udf3v20.cfg
├── resume-cis0kdrjtoj4a0df7kbg.cfg
├── resume-cis0kfjjtoj4a21ooljg.cfg
├── resume-cis0kmbjtoj4a4m92qa0.cfg
├── resume-cisir3rjtoj2j6hvnimg.cfg
├── resume-cisirn3jtoj2k3d2lg00.cfg
└── resume-cit52sjjtoj0p5rhtlr0.cfg
/Users/tarun/.config/nuclei
└── reporting-config.yaml

2 directories, 18 files

====================================

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration does not happen if destination directory does not exist already exists 🤔

$  bash migration.sh nuclei | tee nuclei2.log
Cloning into 'goflags'...
remote: Enumerating objects: 813, done.
remote: Counting objects: 100% (371/371), done.
remote: Compressing objects: 100% (167/167), done.
remote: Total 813 (delta 249), reused 251 (delta 195), pack-reused 442
Receiving objects: 100% (813/813), 6.66 MiB | 18.38 MiB/s, done.
Resolving deltas: 100% (477/477), done.

Checkout os_agnostic_configdir branch
Switched to a new branch 'os_agnostic_configdir'
branch 'os_agnostic_configdir' set up to track 'origin/os_agnostic_configdir'.

Using goflags from /Users/tarun/testing/migration_tests/goflags
Starting nuclei migration
Cloning into 'nuclei'...
remote: Enumerating objects: 34429, done.
remote: Counting objects: 100% (562/562), done.
remote: Compressing objects: 100% (257/257), done.
remote: Total 34429 (delta 331), reused 458 (delta 291), pack-reused 33867
Receiving objects: 100% (34429/34429), 19.97 MiB | 1.53 MiB/s, done.
Resolving deltas: 100% (24016/24016), done.

====================================

====================================
Content of previous config directory
/Users/tarun/.config/nuclei
├── config.yaml
├── reporting-config.yaml
├── resume-cip9isbjtoj1gtd04odg.cfg
├── resume-cip9j0rjtoj1h2044oh0.cfg
├── resume-cipe5qjjtojec3ta4kmg.cfg
├── resume-ciqp8tbjtoj157c79s70.cfg
├── resume-ciqplu3jtoj6nag115ug.cfg
├── resume-cirskijjtoj94ibj1gtg.cfg
├── resume-cirvs7bjtoj5mbth5p00.cfg
├── resume-cis0693jtoj01ibq11h0.cfg
├── resume-cis0k8jjtoj49udf3v20.cfg
├── resume-cis0kdrjtoj4a0df7kbg.cfg
├── resume-cis0kfjjtoj4a21ooljg.cfg
├── resume-cis0kmbjtoj4a4m92qa0.cfg
├── resume-cisir3rjtoj2j6hvnimg.cfg
├── resume-cisirn3jtoj2k3d2lg00.cfg
└── resume-cit52sjjtoj0p5rhtlr0.cfg

1 directory, 17 files

====================================
Staring migration

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v2.9.9

		projectdiscovery.io

[INF] Current nuclei version: v2.9.9 (latest)
[INF] Current nuclei-templates version: v9.5.8 (latest)
[INF] New templates added in latest release: 113
[INF] Templates loaded for current scan: 6424
[INF] No results found. Better luck next time!

====================================
Content of new config directory and old config directory
/Users/tarun/Library/Application Support/nuclei
└── config.yaml
/Users/tarun/.config/nuclei
├── config.yaml
├── reporting-config.yaml
├── resume-cip9isbjtoj1gtd04odg.cfg
├── resume-cip9j0rjtoj1h2044oh0.cfg
├── resume-cipe5qjjtojec3ta4kmg.cfg
├── resume-ciqp8tbjtoj157c79s70.cfg
├── resume-ciqplu3jtoj6nag115ug.cfg
├── resume-cirskijjtoj94ibj1gtg.cfg
├── resume-cirvs7bjtoj5mbth5p00.cfg
├── resume-cis0693jtoj01ibq11h0.cfg
├── resume-cis0k8jjtoj49udf3v20.cfg
├── resume-cis0kdrjtoj4a0df7kbg.cfg
├── resume-cis0kfjjtoj4a21ooljg.cfg
├── resume-cis0kmbjtoj4a4m92qa0.cfg
├── resume-cisir3rjtoj2j6hvnimg.cfg
├── resume-cisirn3jtoj2k3d2lg00.cfg
└── resume-cit52sjjtoj0p5rhtlr0.cfg

2 directories, 18 files

====================================

while this is expected behaviour . what do you think ?? @ehsandeep @Mzack9999

Next Steps

  • Release Major version ?? (since this is breaking change )
  • update config directory in tools (nuclei , subfinder ,alterx etc) which use config dir to store some stuff
  • Add new config path to common healthcheck on all tools (it is not predicatable since new config dir follows convention and is different for most distros and envs)
  • Release Co-ordination with other tools
  • Ensure this does not migrate dependency imports of tool (when merged nuclei should not update config dirs of uncover,tlsx etc)

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dogancanbakir can you please resolve the merge conflict?

@ehsandeep ehsandeep merged commit 065d52b into projectdiscovery:main Sep 29, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use os-agnostic os.UserConfigDir func
4 participants