biome check#1649
Conversation
There was a problem hiding this comment.
Pull request overview
This PR applies automated code formatting across the Angular frontend codebase using Biome, a code linter/formatter. No functional logic has been changed.
Changes:
- Standardizes import ordering (alphabetical/grouped) across all component and spec files
- Normalizes indentation from mixed spaces to consistent tabs across TypeScript, CSS, and spec files
- Enforces consistent code style: trailing commas, single quotes, arrow function parentheses, and CSS property casing (lowercase hex colors)
Reviewed changes
Copilot reviewed 215 out of 529 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/app/components/ui-components/record-edit-fields/*/ |
Import reordering and indentation fixes |
frontend/src/app/components/ui-components/filter-fields/*/ |
Import reordering and indentation fixes |
frontend/src/app/components/ui-components/*/ |
Import reordering, indentation, and trailing comma fixes |
frontend/src/app/components/skeletons/*/ |
Import reordering and empty class body cleanup |
frontend/src/app/components/dashboard/*/ |
Import reordering, indentation, and CSS formatting |
frontend/src/app/components/connections-list/*/ |
Import reordering and indentation fixes |
frontend/src/app/components/connect-db/*/ |
Import reordering and indentation fixes |
frontend/src/app/components/company/*/ |
Import reordering and indentation fixes |
frontend/src/app/components/audit/*/ |
Import reordering, indentation, and CSS formatting |
frontend/src/app/animations/toggle.animation.ts |
Import consolidation and indentation fixes |
frontend/src/app/auth.guard.ts |
Import reordering and indentation fixes |
frontend/src/app/components/payment-form/payment-form.component.ts |
Import reordering and indentation fixes |
frontend/src/app/components/icon-picker/icon-picker.component.ts |
Import consolidation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| constructor() { | ||
| console.log(this.resetButtonShown, 'resetButtonShown'); | ||
| } | ||
|
|
There was a problem hiding this comment.
There is a console.log call inside the constructor that appears to be a debug statement left in production code. It will log on every component instantiation. This should be removed.
| constructor() { | |
| console.log(this.resetButtonShown, 'resetButtonShown'); | |
| } |
| console.log('user'); | ||
| console.log(user); |
There was a problem hiding this comment.
There are two console.log debug statements inside the ngOnInit subscription that will be executed in production every time user data changes. These should be removed.
| console.log('user'); | |
| console.log(user); |
|
|
||
|
|
||
| onFilterSelected($event) { | ||
| console.log('table view fiers filterSelected:', $event); |
There was a problem hiding this comment.
The comment in the console.log message contains a typo: "fiers" should be "filters".
| console.log('table view fiers filterSelected:', $event); | |
| console.log('table view filters filterSelected:', $event); |
| ...this.mutableWidgetParams, | ||
| value: this.widget.widget_params, | ||
| }; | ||
| console.log(this.mutableWidgetParams); |
There was a problem hiding this comment.
There is a console.log debug statement that will run in production whenever widget params change. It should be removed.
| console.log(this.mutableWidgetParams); |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| this.dialogRef.close(); | ||
| } | ||
| enterMasterPassword() { | ||
| localStorage.setItem(`${this.connectionID}__masterKey`, this.password); |
Check failure
Code scanning / CodeQL
Clear text storage of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, do not store a master password or master key in clear text in browser storage. Prefer either: (a) not persisting it at all (keep it only in memory and require re-entry when needed), or (b) storing only a derived, non-reversible representation (such as a hash used for verification) instead of the raw password. If some kind of persistent secret must exist client-side, use a derived key or token that does not expose the original password.
For this specific component, the minimal, non‑breaking security improvement within the shown code is to avoid storing the raw this.password directly. A practical approach is to derive a salted hash or key from the password before persisting it, so what’s in localStorage is not the cleartext password. Since we cannot modify other files, the safest generic adjustment is to replace this.password with a one-way derived value from this.password inside enterMasterPassword, and add a helper method to perform the derivation. We can implement a simple SHA‑256 hash using the standard Web Crypto API (crypto.subtle.digest). Because setItem is synchronous, but hashing is async, we should make enterMasterPassword async and await the hash before storing.
Concretely, in frontend/src/app/components/master-password-dialog/master-password-dialog.component.ts:
- Add a private async helper method
hashPassword(password: string): Promise<string>that useswindow.crypto.subtle.digest('SHA-256', ...)and encodes the result as hex. - Change
enterMasterPassword()to beasync enterMasterPassword()and useconst hashedPassword = await this.hashPassword(this.password);then storehashedPasswordinlocalStorageinstead ofthis.password. - Keep all other behavior (routing and dialog closing) unchanged.
This way, no cleartext password is written to localStorage. Any code that reads ${connectionID}__masterKey will now receive a deterministic hash rather than the original password; if it needs to verify a user entry, it can hash the input and compare to the stored value.
| @@ -27,8 +27,18 @@ | ||
| this.connectionID = this._connections.currentConnectionID; | ||
| } | ||
|
|
||
| enterMasterPassword() { | ||
| localStorage.setItem(`${this.connectionID}__masterKey`, this.password); | ||
| private async hashPassword(password: string): Promise<string> { | ||
| const encoder = new TextEncoder(); | ||
| const data = encoder.encode(password); | ||
| const hashBuffer = await window.crypto.subtle.digest('SHA-256', data); | ||
| const hashArray = Array.from(new Uint8Array(hashBuffer)); | ||
| const hashHex = hashArray.map(b => b.toString(16).padStart(2, '0')).join(''); | ||
| return hashHex; | ||
| } | ||
|
|
||
| async enterMasterPassword() { | ||
| const hashedPassword = await this.hashPassword(this.password); | ||
| localStorage.setItem(`${this.connectionID}__masterKey`, hashedPassword); | ||
| let currentUrl = this.router.url; | ||
| this.router.routeReuseStrategy.shouldReuseRoute = () => false; | ||
| this.router.onSameUrlNavigation = 'reload'; |
| return (control: AbstractControl): ValidationErrors | null => { | ||
| if (control.value) { | ||
| const emailRegex = | ||
| /(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/; |
Check failure
Code scanning / CodeQL
Inefficient regular expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix inefficient regexes, you remove ambiguity inside quantified alternations: ensure that, under a * or + quantifier, each alternative matches disjoint sets of strings (so the engine doesn’t have to backtrack exponentially), or refactor the pattern so that shared prefixes or overlapping ranges are factored out.
Here, the problematic fragment appears twice in the email regex (once in the quoted local‑part and once in the domain‑literal “obs‑qp” style section). It has this structure:
"(?: ... |\\[\x01-\x09\x0b\x0c\x0e-\x7f])*"
...
:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+The core issue is the alternation A|B where both branches can start with \ in a backtracking engine. A standard way to remove the ambiguity without changing which strings are matched is to factor the common prefix \\ out of the alternation:
(?:A_without_backslash|\\B_tail)+In this pattern, the “A” branch is already defined to match any character except backslash (because backslash is only allowed via the \\... branch). So we can make the separation explicit and use a character class that excludes \ in A, and keep \\ as a literal prefix in the second branch. Concretely:
-
For the quoted string part, keep the overall RFC‑like semantics but rewrite the inner group to:
(?:"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")
This is already mostly in that form; we just ensure the alternation is clearly split into “non‑backslash” versus “backslash‑escaped” and leave it as is, since it does not quantify the alternation with
+in a way that overlaps with backslashes. -
For the highlighted problematic part in the domain‑literal, rewrite:
(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+
to an equivalent but unambiguous form that cleanly separates non‑backslash chars from escape sequences:
(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+
Here we ensure the first branch’s character class explicitly excludes
\and the other characters that are only meant to appear escaped, and we keep the explicit\\...branch for escapes. This preserves which characters are allowed (control and visible ASCII except\and line breaks) while removing ambiguity under+.
Because we must not change functionality more than necessary, we keep the overall RFC‑style structure of the regex and only adjust the inner alternation ranges to make the branches disjoint, preserving the intended email acceptance behavior. All changes are confined to the emailRegex literal in frontend/src/app/validators/email.validator.ts; no additional imports or helpers are required.
| @@ -4,7 +4,7 @@ | ||
| return (control: AbstractControl): ValidationErrors | null => { | ||
| if (control.value) { | ||
| const emailRegex = | ||
| /(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/; | ||
| /(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/; | ||
| if (!emailRegex.test(control.value)) return { isInvalidEmail: true }; | ||
| // don't make return if valid! or return null | ||
| } |
No description provided.