rewinged icon indicating copy to clipboard operation
rewinged copied to clipboard

added option to log the download if there is sqlite database

Open bennyc-huji opened this issue 1 year ago • 1 comments

added option to log the download if there is sqlite database and stats environment. when you set a variable "stats" or create it in the json, it will automatically will create a database that checks the download of the files in the installer the files in the installer must be in this format: VENDOR.PACKAGE.v.0.0.0.0.EXT

bennyc-huji avatar Jun 30 '24 09:06 bennyc-huji

Hi, thank you for the PR.

However I see some problems and have some questions, mainly:

  1. The AutoInternalize feature names all installlers by their SHA256 checksum, to avoid name conflicts, so requiring VENDOR.PACKAGE.v.0.0.0.0.EXT won't work. See: https://github.com/jantari/rewinged/blob/9a619c5d27c67469a27259535d95bd8ff001c6d6/manifests.go#L153
  2. What is the benefit of adding a SQLite DB for log entries like this? Currently rewinged, like most software, logs to stdout and can do so in structured JSON format. This already allows for easy analyzing of the logs and package downloads, and stdout logs can be sent to SIEMs or syslog servers - a SQLite DB cannot.
  3. Splitting user-defined strings (like installer names) on regex expressions without error handling will always result in errors, crashes or potentially even vulnerabilities so I'm against parsing installer names like that
  4. I'm not sure the program logic can assume that a package download is happening whenever a requests param.BodySize is equal to 2 - surely there are other kinds of requests with a bodysize of 2? Or package downloads with a different bodysize? A user could curl / request anything after all. This would crash the server or corrupt the log.

I'm not sure whether there is a sure way for rewinged to know when a client has initiated a package install. The installer files could be hosted elsewhere, outside of rewinged. And just requesting the manifest does not necessarily indicate that a client started an install.

jantari avatar Jul 01 '24 20:07 jantari

I won't be merging this as-is over the aforementioned concerns, but feel free to fork.

jantari avatar Aug 24 '24 01:08 jantari