Skip to content

add security.md#439

Open
shawn0915 wants to merge 1 commit into
iflytek:mainfrom
shawn0915:patch-1
Open

add security.md#439
shawn0915 wants to merge 1 commit into
iflytek:mainfrom
shawn0915:patch-1

Conversation

@shawn0915
Copy link
Copy Markdown

Summary

  • What changed?

Add security.md file.

  • Why is this needed?

To allow raise issue about security.

Validation

  • Backend tests passed
  • Frontend typecheck/build passed
  • OpenAPI SDK regenerated or checked when API contracts changed
  • Smoke test run when relevant

Commands run:

# paste commands here

Risk

  • User-facing impact:
  • Deployment or migration impact:
  • Rollback approach:

Notes

  • Related issue:
  • Follow-up work:
  • Docs or operator runbooks updated when behavior changed:

Signed-off-by: Shawn Yan <shawn2016@aliyun.com>
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a comprehensive SECURITY.md file that details the project's security architecture, vulnerability reporting process, and deployment security checklist. Feedback from the review highlights the need to provide a direct security contact email instead of referring to an incomplete CODE_OF_CONDUCT.md and recommends removing the hardcoded default bootstrap password from the public documentation to mitigate potential security risks.

Comment thread SECURITY.md
To report a security issue, use one of the following private channels:

- **GitHub Security Advisories**: [https://github.com/iflytek/skillhub/security/advisories/new](https://github.com/iflytek/skillhub/security/advisories/new)
- **Email**: Send a detailed report to the maintainers via private channels listed in the repository's `CODE_OF_CONDUCT.md`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

The instruction to report vulnerabilities via email by referring to CODE_OF_CONDUCT.md is currently broken because the provided CODE_OF_CONDUCT.md does not contain any email addresses or specific contact details. It only mentions a "private maintainer channel" without defining how to access it. This creates a barrier for security researchers. Please provide a direct security email address or update CODE_OF_CONDUCT.md with the necessary contact information.

Suggested change
- **Email**: Send a detailed report to the maintainers via private channels listed in the repository's `CODE_OF_CONDUCT.md`
- **Email**: [Insert security contact email address]

Comment thread SECURITY.md
The `BootstrapAdminInitializer` creates a default admin account on first startup when `BOOTSTRAP_ADMIN_ENABLED=true` (the default for Docker-based deployments). The default credentials are:

- Username: `admin`
- Password: `ChangeMe!2026`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Hardcoding the default bootstrap password in the public SECURITY.md file is a security risk, as it makes it trivial for attackers to find and attempt to use these credentials against unconfigured instances. It is safer to refer to the .env.release.example file or an environment variable (e.g., BOOTSTRAP_ADMIN_PASSWORD) without disclosing the actual default string in the security policy.

Suggested change
- Password: `ChangeMe!2026`
- Password: See the default value in .env.release.example

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