Skip to content

Conversation

@alowave
Copy link
Contributor

@alowave alowave commented Feb 2, 2025

No description provided.

@shdwmtr shdwmtr closed this Feb 5, 2025
@shdwmtr shdwmtr reopened this Feb 5, 2025
@shdwmtr
Copy link
Member

shdwmtr commented Feb 5, 2025

Ignore me closing and opening this I'm testing something lol.

@shdwmtr shdwmtr closed this Feb 5, 2025
@shdwmtr shdwmtr reopened this Feb 5, 2025
@shdwmtr shdwmtr closed this Feb 5, 2025
@shdwmtr shdwmtr reopened this Feb 5, 2025
@shdwmtr
Copy link
Member

shdwmtr commented Feb 5, 2025

Okay I'm done testing stuff.

Some notes before I can merge this,

  1. Your repository content should be top level meaning

    • package.json
    • plugin.json
    • requirements.txt
    • tsconfig.json

    Should be in the root / directory of the repository, not nested under /src

  2. Another note, you should add **/.millennium to your .gitignore. It's a security risk to have built plugin assets within the repository.

@alowave
Copy link
Contributor Author

alowave commented Feb 5, 2025

Thank you for your feedback.
Which kind of security risk is this? Should I re-create the repository, or it will be fine to just add it to gitignore and exclude from the repo?
Regards part 1, I will fix it asap.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 5, 2025

Should I re-create the repository, or it will be fine to just add it to gitignore and exclude from the repo?

Make sure your repository has no outgoing changes, then add it to your .gitignore file, and run

git rm -rf --cached .
git add .

This should remove the files from repo.

Which kind of security risk is this?

Having built assets in your repository is somewhat of a grey-area when it comes to open source. If they are built, they are somewhat obfuscated and not as readable. This can be used by bad actors to make changes in the built assets they don't want users to see. Not only that, but most people wouldn't recognize the built files and would look solely at the source code, not recognizing it isn't a valid reflection of what the actual build files might do.

i.e if in your source code you have
a = 1
but your build files have
a = 2

its not an accurate representation of what your plugin does. It also makes it harder to audit plugin updates, as we'd have to check two places

@alowave alowave closed this Feb 6, 2025
@alowave alowave reopened this Feb 6, 2025
@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

I fixed everything you mentioned. Thank you for the explanation in security part.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Your plugin submodule is out of date, you didn't update it after making new commits.

https://github.com/shdwmtr/plugdb?tab=readme-ov-file#updating-your-plugin

- Remove of .millennium
- Move source to main tree of repo.
@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Fixed, sorry, for some reason git didn't push the changes.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Your build is failing. Take a look

@shdwmtr shdwmtr closed this Feb 6, 2025
@shdwmtr shdwmtr reopened this Feb 6, 2025
I hope.
@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Thank u and sorry for time spent. Now should be fine.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

A lot closer to working, but still isn't. You need to change dist to .millennium/Dist here. I didn't notice the error initially.
Good thing automated checks exist to verify everything.

@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Thank you, this is due to new building system i guess. Before i push the change - how can i remove frontend from my plugin? I have build errors that this is not found when trying to delete the folder. I have not noticed any mention of this file in build scripts.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Also note that your build instructions in your README are wrong, its the instructions to build the example plugin, not yours.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Thank you, this is due to new building system i guess. Before i push the change - how can i remove frontend from my plugin? I have build errors that this is not found when trying to delete the folder. I have not noticed any mention of this file in build scripts.

I'm not sure if its supported in the public build. I'll add support in the future.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Perfect, make sure to test the build to make sure it works as expected:
https://github.com/shdwmtr/plugdb/actions/runs/13182019307?pr=15 (the builds are at the bottom, scroll down)

@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Using the build provided steam is failing to inject shims. The login window is paused without any further movement.
Logs are:
https://pastebin.com/jN3TQzEW

@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Using the build provided steam is failing to inject shims. The login window is paused without any further movement. Logs are: https://pastebin.com/jN3TQzEW

Also, there is no static folder in build provided.

@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Using the build provided steam is failing to inject shims. The login window is paused without any further movement. Logs are: https://pastebin.com/jN3TQzEW

This was fixed by magic, have no idea what i've done. However, now, after adding static folder everything runs. But plugin is not functional due to missing of webkit in build.

So, for now there are 2 issues:

  1. Missing static folder
  2. Missing webkit

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

add

{
    ...
    "include": ["webkit", "static"]
}

to your plugin.json. This lets the CI know you want those files included in the build.

Edit: Do you mean the webkit folder or the webkit.js file

Add components (webkit,static) to plugin.json
@alowave
Copy link
Contributor Author

alowave commented Feb 6, 2025

Hi

add

{
    ...
    "include": ["webkit", "static"]
}

to your plugin.json. This lets the CI know you want those files included in the build.

Edit: Do you mean the webkit folder or the webkit.js file

I mean webkit.js file, there is no such file in the build. in C:\Program Files (x86)\Steam\plugins\faceit-stats\.millennium\Dist
Edit:
tsconfig.json:
"include": ["webkit/index.ts"],
package.json:
"main": "./webkit/index.ts",

@shdwmtr
Copy link
Member

shdwmtr commented Feb 6, 2025

Try building it locally, does it have a webkit.js?

Fixing build one more and i hope last time.
.ts -> .tsx
@alowave alowave closed this Feb 7, 2025
@alowave alowave reopened this Feb 7, 2025
@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

Pull changes from plugdb, its not the same as yours.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

Make sure it works

@alowave
Copy link
Contributor Author

alowave commented Feb 8, 2025

Build is tested and works as expected.

@shdwmtr shdwmtr merged commit 483fae4 into SteamClientHomebrew:main Feb 8, 2025
7 checks passed
@alowave
Copy link
Contributor Author

alowave commented Feb 8, 2025

Thank you. Sorry that i've spent so much of your time fixing minor issues.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

Should be on the plugin page in ~30 minutes max, just need to wait for the cache to expire.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

No worries; good thing the CI caught the errors instead of end-users.

@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

Your perma-link for the plugin is https://steambrew.app/plugin?id=57c553750f61.

Let me know if you're ever going to move repositories in advance. This doesn't include username change, repo name change, moving the repo to an organization, etc. Just if you actually make a new repo and put it there / delete the current one.

The reason its important is because the id is based off the initial commit id of the repo, i.e 57c553750f616459005a996e765c6bc582a1e803

@shdwmtr
Copy link
Member

shdwmtr commented Feb 8, 2025

You may also want to change the install instructions, as most people will be viewing them from steambrew.app and not GitHub, so they don't follow correctly.

@alowave
Copy link
Contributor Author

alowave commented Feb 8, 2025

You may also want to change the install instructions, as most people will be viewing them from steambrew.app and not GitHub, so they don't follow correctly.

I was waiting for link to update the README.md, thank you for providing it. I will update instructions ASAP.

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.

2 participants