Leask / halbot

Just another `ChatGPT` / `Gemini` / `Mistral (by ollama)` Telegram bob, which is simple design, easy to use, extendable and fun.
https://leaskh.com/post/711636926789271552/halbot
MIT License
101 stars 16 forks source link

about “.halbot.json” #30

Closed mccoysc closed 1 year ago

mccoysc commented 1 year ago
  1. 配置文件写成绝对路径是个非常坏的习惯:太多的不可预料因素导致这个绝对路径 不存在、不允许被访问、不允许修改、不允许被读、不允许被写 等等异常情况且没有解决方案或者解决起来很复杂
  2. 敏感数据(比如key)写到文件文件是个非常坏的习惯,比如我大概猜测,之所以你这里没有自带config.json是因为这种敏感数据你不方便公开,但是对于多数开发者而言,一旦写到文件里,与github同步时,一不小心就会把这种敏感文件同步了,这是给信息泄露增加可能性。

建议改为环境变量,比如 process.env.BOT_CONFIG,然后把所需的json字符串作为这个环境变量的值,初始化时:

options=JSON.parse(process.env.BOT_CONFIG||"{}");

或者,如果你非常喜欢用配置文件的方式,你也可以这样:

const configFilePath=process.env.BOT_CONFIG_PATH;
const configFileContent=(fs.readFileSync(configFilePath)?.toString()) || "{}";
options=JSON.parse(configFileContent);

Writing the configuration file as an absolute path is a very bad habit: there are too many unpredictable factors that can cause exceptions such as the absolute path not existing, not being accessible, not being modifiable, not being readable, not being writable, etc., and there may be no solution or the solution may be very complicated.

Writing sensitive data (such as keys) to a file is also a very bad habit. For example, I guess the reason why you don't have a built-in config.json file here is that you don't want to expose this sensitive data publicly. However, for most developers, once the sensitive data is written to a file and synchronized with Github, it is easy to accidentally sync this sensitive file, which increases the possibility of information leakage.

It is recommended to use environment variables instead, such as process.env.BOT_CONFIG, and set the required JSON string as the value of this environment variable. During initialization:

options=JSON.parse(process.env.BOT_CONFIG||"{}");

Alternatively, if you prefer to use a configuration file, you can do it like this:

const configFilePath=process.env.BOT_CONFIG_PATH;
const configFileContent=(fs.readFileSync(configFilePath)?.toString()) || "{}";
options=JSON.parse(configFileContent);
Leask commented 1 year ago
  1. 此項目並沒有把配置文件寫成絕對路徑,項目配置文件在 ~/.xxxx 下,這是 Unix/Linux 程序的標準做法,歷史悠久,而且有一定傳統,此項目想保留這一點。
  2. 敏感數據寫到配置文件有討論的餘地,但是這裡假設的都是可控環境,而且項目是個人用途,部署的可控環境以及docker等因素,其實完全可以自己影射文件的位置(docker鏡像很快就會提供了)。並不存在開發者同步配置文件的可能性,因為配置文件並不在本項目的文件夾中,項目也不會在當前文件夾找配置文件。
  3. 改為環境變量我不同意,但是支持通過環境變量來獲取配置的確是一個考慮,我其實也是計畫支持的。
  4. github 的一個好處是fork,遇到和上游開發者意見不認同很正常,我也歡迎你fork然後提供更好或者融合的解決方案,如果我覺得fork的版本更好,當然也歡迎pr回來。

鑑於以上幾點,我暫時先關閉此issue,歡迎繼續討論,感謝關注。

mccoysc commented 1 year ago
  1. 此項目並沒有把配置文件寫成絕對路徑,項目配置文件在 ~/.xxxx 下,這是 Unix/Linux 程序的標準做法,歷史悠久,而且有一定傳統,此項目想保留這一點。
  2. 敏感數據寫到配置文件有討論的餘地,但是這裡假設的都是可控環境,而且項目是個人用途,部署的可控環境以及docker等因素,其實完全可以自己影射文件的位置(docker鏡像很快就會提供了)。並不存在開發者同步配置文件的可能性,因為配置文件並不在本項目的文件夾中,項目也不會在當前文件夾找配置文件。
  3. 改為環境變量我不同意,但是支持通過環境變量來獲取配置的確是一個考慮,我其實也是計畫支持的。
  4. github 的一個好處是fork,遇到和上游開發者意見不認同很正常,我也歡迎你fork然後提供更好或者融合的解決方案,如果我覺得fork的版本更好,當然也歡迎pr回來。

鑑於以上幾點,我暫時先關閉此issue,歡迎繼續討論,感謝關注。

“~/.xxxx”在实际中就是会被解释为一个绝对路径,且解释到哪里是不定的,在不定的环境里,其路径accessable是不可控的。其实这么做就是把路径控制交给了系统,使得你的工程依赖于不可控的系统配置。

任何工程,尽可能降低第三方依赖是原则。

Leask commented 1 year ago

抱歉,第一,說實話我不是很喜歡你交流問題的態度,第二,作為一個從業多年的開發者,我知道應該在什麼級別的系統中如何處理配置文件,並且我喜歡這一傳統。請諒解,接下來我不會再回覆關於配置文件地址的討論,但是支持env或者更豐富的命令行參數的確是在開發計畫中,請期待後續版本。

mccoysc commented 1 year ago

那个配置文件的路径是写在哪个文件的?没看到在哪里,所以也无从修改