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

the relative path in secretFile is error-prone; 限制 secretFile 能访问的路径 #631

Closed
seeflood opened this issue Jun 7, 2022 · 10 comments

Comments

@seeflood
Copy link
Member

seeflood commented Jun 7, 2022

What happened:
Currently we have secretFile configuration in config.json.

"secretsFile": "../../configs/secret/config_secret_local_file.json"

This configuration uses relative path, which is error-prone.

  • Case 1. Success
cd ${project_path}/cmd/layotto
go build -o layotto
./layotto start -c ../../configs/config_standalone.json
  • Case 2. An error occurs if you run layotto under some other directory
# under project root path
go build -o layotto cmd/layotto

# run layotto, but the current path is different from the quickstart doc
./layotto start -c configs/config_standalone.json

error message:
image

  • Case 3. Security issues
    If someone configurates some other path as the secretFile, security issues may arise.
    For example, we can configurate a path where we don't have read permission, and leverage layotto to read the sensitive data.

What you expected to happen:
Restrict that the secretFile configuration can only refer to the files under same path.
For example:

 "secretsFile": "secret_local_file.json" 

And Layotto will try to read the secret_local_file.json under the same path with config.json

How to reproduce it (as minimally and precisely as possible):
See case 2 above

Anything else we need to know?:

@seeflood seeflood added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 7, 2022
@akkw
Copy link
Contributor

akkw commented Jun 8, 2022

这个我来看一下吧

@seeflood
Copy link
Member Author

seeflood commented Jun 8, 2022

@akkw Cool. Thanks!

Update: since @akkw doesn't have time to do it, this issue is free and waiting for assignment

@seeflood seeflood assigned akkw and unassigned akkw Jun 8, 2022
@seeflood seeflood changed the title the relative path in secretFile is error-prone the relative path in secretFile is error-prone 限制 secretFile 能访问的路径 Jun 14, 2022
@seeflood seeflood changed the title the relative path in secretFile is error-prone 限制 secretFile 能访问的路径 the relative path in secretFile is error-prone; 限制 secretFile 能访问的路径 Jun 15, 2022
@MichaelDeSteven
Copy link
Contributor

plz assign it to me, thx

@seeflood
Copy link
Member Author

@MichaelDeSteven Cool. Thanks!

@MichaelDeSteven
Copy link
Contributor

@seeflood 这个是不是要将secret_local_file.json文件位置限制在config.json所在的目录(configs目录),如果不满足条件就抛出错误?

@seeflood
Copy link
Member Author

seeflood commented Jun 24, 2022

@MichaelDeSteven 是的,如果发现不在相同目录就报错
另外,可以允许用户配相对路径,比如

 "secretsFile": "secret_local_file.json" 

就是读 “和配置文件相同目录下的 secret_local_file.json”

@MichaelDeSteven
Copy link
Contributor

MichaelDeSteven commented Jun 24, 2022

@MichaelDeSteven 是的,如果发现不在相同目录就报错

image
我打算这样做

  1. json文件secretsFile字段改为"secretsFile": "secret_local_file.json"
  2. 2.1. create the component之前先检查secretsFiles是否存在。具体做法是start layotto时获取config.json路径参数,再提取前缀与secretsFile文件名拼接,然后判断文件是否存在,不存在就报错。

@Xunzhuo
Copy link
Member

Xunzhuo commented Mar 6, 2023

/good-first-issue cancel
/help-wanted cancel

@github-actions github-actions bot removed good first issue Good for newcomers help wanted Extra attention is needed labels Mar 6, 2023
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

This issue has been automatically marked as stale because it has not had recent activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue or help wanted) or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 6, 2023
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue or help wanted. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants