Skip to content

Conversation

@Marc-Andrieu
Copy link
Member

See the commits for a (rather) clear-cut view of the changes

@Marc-Andrieu Marc-Andrieu self-assigned this Sep 21, 2025
@Marc-Andrieu Marc-Andrieu added documentation Improvements or additions to documentation help wanted Extra attention is needed dependencies Pull requests that update a dependency file core python Pull requests that update Python code ready for review This PR is ready to be reviewed jinja code quality labels Sep 21, 2025
@gitguardian
Copy link

gitguardian bot commented Sep 21, 2025

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
21468011 Triggered RSA Private Key ba94061 config.template.yaml View secret
21445366 Triggered Generic Password 88d9e6d config.template.yaml View secret
7298668 Triggered Generic High Entropy Secret ba94061 config.template.yaml View secret
21445367 Triggered Company Email Password 88d9e6d config.template.yaml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@codecov
Copy link

codecov bot commented Sep 21, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.84%. Comparing base (d7de4da) to head (731e58f).

Files with missing lines Patch % Lines
app/utils/state.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #845      +/-   ##
==========================================
+ Coverage   85.79%   85.84%   +0.04%     
==========================================
  Files         189      189              
  Lines       14597    14590       -7     
==========================================
+ Hits        12524    12525       +1     
+ Misses       2073     2065       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Marc-Andrieu Marc-Andrieu force-pushed the update/readme branch 3 times, most recently from e40d0e6 to 9f44cbb Compare October 7, 2025 21:15
@gitguardian
Copy link

gitguardian bot commented Oct 10, 2025

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

# - <UUID value 2 of a GroupType>
# ```
# Group UUIDs should be values of the GroupType enum from `app.core.groups.groupe_type.GroupType`
FACTORIES_DEMO_USERS:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should have default values for this field. There may be a risk that someone forget to remove them, and thus deploy an api with to exposed accounts.

Comment on lines 6 to 7
# If you want to generate a 2048-bit long PEM certificate and save it in a file, the following command may be used:
# openssl req -newkey rsa:2048 -nodes -keyout key.pem -x509 -days 365 -out certificate.pem
Copy link
Member

Choose a reason for hiding this comment

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

Is this the doc for RSA_PRIVATE_PEM_STRING?

Copy link
Member

Choose a reason for hiding this comment

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

Idem


## 2. Install dependencies

### About Jellyfish and Rust
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is needed for macOS

Comment on lines +123 to +127
It is a binary.
This means:

- SQLite is lightweight
- It is directly understood by your machine, no special configuration is needed.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this will be understandable for the majority of people

README.md Outdated

## Create the first user
1. `ACCESS_TOKEN_SECRET_KEY`: **Uncomment it**.
You can generate your own if you want, or just change a couple characters, or leave it as it is.
Copy link
Member

Choose a reason for hiding this comment

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

You should never use a publicy exposed signature secret. This readme may be used by someone who want to configure a public facing production Hyperion.

Event for a development configuration, in some situations it could be a bad idea to use a default secret.

Comment on lines +10 to +11
# If you want to generate a 2048-bit long PEM certificate and save it in a file, the following command may be used:
# openssl req -newkey rsa:2048 -nodes -keyout key.pem -x509 -days 365 -out certificate.pem
Copy link
Member

Choose a reason for hiding this comment

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

You should put the last two lines after:

# RSA_PRIVATE_PEM_STRING should be a string containing the PEM certificate of a private RSA key. It will be used to sign id_tokens for Openid connect authentication
# The example below was generated using a 2048-bit RSA key generator

POSTGRES_HOST="hyperion-db"
POSTGRES_USER=""
POSTGRES_PASSWORD=""
POSTGRES_HOST=""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
POSTGRES_HOST=""
# Should be set to the name of the postgres container
POSTGRES_HOST=""

Comment on lines 6 to 7
# If you want to generate a 2048-bit long PEM certificate and save it in a file, the following command may be used:
# openssl req -newkey rsa:2048 -nodes -keyout key.pem -x509 -days 365 -out certificate.pem
Copy link
Member

Choose a reason for hiding this comment

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

Idem


# ACCESS_TOKEN_SECRET_KEY should contain a random string with enough entropy (at least 32 bytes long) to securely sign all access_tokens for OAuth and Openid connect
ACCESS_TOKEN_SECRET_KEY: ""
ACCESS_TOKEN_SECRET_KEY: #YWZOHliiI53lJMJc5BI_WbGbA4GF2T7Wbt1airIhOXEa3c021c4-1c55-4182-b141-7778bcc8fac4
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how this value was generated, but I wonder if someone could have concatenated a random sequence with an uuid. I suggest using a value that is really a random sequence, as one could argue that uuid may not be suitable to use as secret.

Marc-Andrieu and others added 15 commits October 19, 2025 15:39
## Description

### Summary

<!--Brief description of what this PR does.-->

Bug: his unexpected comment explains why new users don't have their
avatar imported through our SSO to the Matrix server.

Feature: it seems the MAS (Matrix Authentication Service) now supports
the `picture` claim of the OIDC scope, so we add it [back].

Source:
https://github.com/element-hq/matrix-authentication-service/blob/1bd1b00524a62e365cf06b3745d298a53df69c90/crates/jose/src/claims.rs#L525

## Type of Change

We present it as a feature but it's actually a bug fix...

- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [x] ✨ New feature (non-breaking change which adds functionality)
- [ ] 🔨 Refactor (non-breaking change that neither fixes a bug nor adds
a feature)
- [ ] 🔧 Infra CI/CD (changes to configs of workflows)
- [ ] 💥 BREAKING CHANGE (fix or feature that require a new minimal
version of the front-end)

## Impact & Scope

- [ ] Core functionality changes
- [ ] Single module changes
- [ ] Multiple modules changes
- [ ] Database migrations required
- [x] Other: 3rd party service integration

## Testing

- [ ] Added/modified tests that pass the CI
- [ ] Tested in a pre-prod
- [ ] Tested this locally
- [x] Will be tested in prod directly...

## Documentation

- [ ] Updated docs accordingly (docs.myecl.fr) : <!--[Docs#0 -
Title](https://github.com/aeecleclair/myecl-documentation/pull/0)-->
- [ ] Code includes docstrings
- [x] No documentation needed
# Conflicts:
#	.github/workflows/publish.yml
#	.github/workflows/publishbase.yml
#	Dockerfile
#	Dockerfile.base
#	pyproject.toml
#	requirements.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality core dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation help wanted Extra attention is needed jinja python Pull requests that update Python code ready for review This PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants