Merge release/1.5-alpha into main #1541
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* pass device management flag in enrollment settings * return error if device management is disabled * add admin flag to enrollment response * update protos * update protos --------- Co-authored-by: Maciej Wójcik <[email protected]>
* VPN crud events * Emit VpnLocationAdded event when importing from file * ApiToken event handling * OpenId client app events * OpenIdProvider events * Settings events * Restore default settings event * Group management events * WebHook events * Webauthn key management events * AuthenticationKey events * Password change, enrollment events * Translation * Store whole objects in the events * Don't store secrets in metadata * Implement UserNoSecrets metadata struct * Update sqlx query data * Remove skip_serializing marker * Box event enums to avoid big size differences between variants * Allow large enum variant on GatewayServerError * Don't box whole events * Remove todo comment * Remove client events
* oidc mfa in client * cargo fix * sqlx, fixes * fmt * prevent using oidc mfa if the provider is not configured * revert i18n types * review changes * invalidate user on openid fail * update protos * fmt fix * sqlx * dont log failed precondition errors
* bind address * clippy
Merge main -> dev post 1.4 release
* update dependencies * move file to correct module * allow using IpAddr in models * add table for SNAT bindings * add bruno API collection * set up basic list endpoint * add basic binding management API * delete API docs * add API docs * update protos * send snat bindings to the gateway * update query data * make snat API endpoints enterprise-only * trigger firewall update on snat binding change * update protos * update protos * update dependencies * Update crates/defguard_core/src/enterprise/firewall/mod.rs Co-authored-by: Aleksander <[email protected]> * move snat model definitions * Update crates/defguard_core/src/enterprise/firewall/mod.rs Co-authored-by: Adam <[email protected]> * remove unused variable --------- Co-authored-by: Maciej Wójcik <[email protected]> Co-authored-by: Aleksander <[email protected]> Co-authored-by: Adam <[email protected]>
* allow admin to disable users MFA * allow disabling MFA from list view * change button style --------- Co-authored-by: Maciej Wójcik <[email protected]>
Fix auth key delete endpoint
* update rust edition * update dependencies * adjust code * run new formatter * post-merge fixes --------- Co-authored-by: Maciej Wójcik <[email protected]>
* move migrations directory * update build script * update dockerfile * fix migrations in tests --------- Co-authored-by: Maciej Wójcik <[email protected]>
* add extractor to get relevant location feat: add Path extractor to get location_id from URL path in from_request_parts add extractor to get relevant location * add 404 errors to remaining endpoints * actually handle bindings already existing * formatting * update dependencies * remove duplicate method * pin rust-ini version * add integration tests for SNAT API * formatting --------- Co-authored-by: Maciej Wójcik <[email protected]>
* add test qr gen for mobile client * mobile client qr for dev
* add description column to activity log events * setup a system for generating event descriptions * add messages to existing logs * restore missing code path * add description column to event list * update query data * avoid unnecessary queries when logged in as non-admin user * formatting * add events for snat binding management * add event for disabling user MFA * add more descriptions * add vpn events descriptions * add more descriptions * fill in remaining descriptions * linter fixes * reduce column widths * add search by description * formatting --------- Co-authored-by: Maciej Wójcik <[email protected]>
* redesign add device page * up packages * hide merged token * update translation * change client download link
* fix logout when disabled e2e test * fix lint
* fix logout when disabled e2e test * fix lint * replace eslint with biome fix tsc checking * web node version from 23 to 24 * fix outside component hook usage
* update dependencies * add location name column * add location to event context * show location in activity log page * allow filtering, sorting and searching by location * implement filters * update query data * include location in grpc request context * simplify context conversion for grpc events * adjust enum variant sizes --------- Co-authored-by: Maciej Wójcik <[email protected]>
* update dependencies * add new location MFA config to DB model * update location struct to include new mfa field * update API to expect MFA type * fix translation for network device setup * add temporary frontend for setting location MFA type * update protos * handle updated protos in code * remove openid mfa setting from frontend * update query data * update protos * handle restored field * handle updated field naming * handle removal of openid provider * update query data * bump version to 1.5.0 * validate correct MFA method is selected * remove unused transaction * update test network fixtures * remove remaining references to mfa_enabled * skip e2e tests until final UI is implemented --------- Co-authored-by: Maciej Wójcik <[email protected]>
* update dependencies * add new location MFA config to DB model * update location struct to include new mfa field * update API to expect MFA type * fix translation for network device setup * add temporary frontend for setting location MFA type * update protos * handle updated protos in code * remove openid mfa setting from frontend * update query data * update protos * handle restored field * handle updated field naming * handle removal of openid provider * update query data * bump version to 1.5.0 * validate correct MFA method is selected * remove unused transaction * update test network fixtures * remove remaining references to mfa_enabled * skip e2e tests until final UI is implemented * add styled MFA mode select * add message box and section header * reenable e2e openid tests * fix mfa e2e test * formatting --------- Co-authored-by: Maciej Wójcik <[email protected]>
* update dependencies * add label to mode select * move peer disconnect threshold into mfa section * move threshold below MFA mode * rearrange sections * avoid incorrect gateway status requests * update frontend dependencies --------- Co-authored-by: Maciej Wójcik <[email protected]>
* update user disable test * add failing test case * remove API tokens when deactivating a user * only accept tokens from active users
* add test for dg25-19 * tests don't have frontend built
* check failed logins before proceeding with TOTP verification * use username in failed login map whenever possible * log failed code verification attempts * add tests for brute force login
* add failing test * simplify API routes setup * restrict device management endpoints
* add failing test * simplify API routes setup * restrict device management endpoints * add failing test * restrict device config access
* properly respect openid scopes * fix
* clear authorized apps after changing scope * fix * remove redundant header * prepare
Implements incompatible version notifications in core UI. Related issue: #1298.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| // NOTE: After waiting, sleep for 3 seconds to let Defguard Core apply migrations. | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done; sleep 3'`; | ||
| execSync(wait_for_db); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, avoid constructing shell commands with dynamic values embedded in command strings. Instead, use argument arrays and pass them to execFileSync or execFile so the shell is not involved in parsing these values.
Specifically:
- In
e2e/utils/docker.ts, replace uses ofexecSync(command)(wherecommandincludes dynamic/tainted paths) withexecFileSyncorexecFile. - Parse out the command and arguments explicitly. For example, instead of
"docker compose -f <file> up --wait", useexecFileSync('docker', ['compose', '-f', dockerFilePath, 'up', '--wait']);. - This pattern should be followed for all instances where
dockerComposeis used as part of a command string. - Import
execFileSyncfrom'child_process'. - Do not change the semantics (e.g., continue to use synchronous execution unless there is a technical reason to change).
- Duplicate the edit pattern for all
execSync(command)that rely ondockerCompose.
Required changes:
- Replace each shell string construction/
execSyncusage with argument array version usingexecFileSync. - Import
execFileSyncfrom'child_process'. - Remove or adapt
dockerComposeso onlydockerFilePathis reused; do not store or construct commands as strings.
Suggested changeset
1
e2e/utils/docker.ts
Copilot is powered by AI and may make mistakes. Always verify output.
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done; sleep 3'`; | ||
| execSync(wait_for_db); | ||
| const create_snapshot = `${dockerCompose} exec db pg_dump -U defguard -Fc -f /tmp/defguard_backup.dump defguard`; | ||
| execSync(create_snapshot); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix this problem, replace all usages of execSync that invoke Docker Compose commands constructed via string concatenation with usages of execFileSync and arguments provided as an array. This ensures file paths and other dynamic values are not interpreted by the shell, eliminating the risk of command injection and unexpected behavior (e.g., spaces, quotes, etc.).
Specifically:
- On lines constructing commands (e.g., line 17 for
create_snapshot), break the command into executable and argument array form. - Replace
execSync(commandString)withexecFileSync(executable, args). - For
docker-composecommands, split the command into executable'docker'and args['compose', '-f', dockerFilePath, ...]. - For commands with sub-commands,
exec, and complex shell invocations likesh -c, pass inner scripts as arguments, always keeping user-controlled file paths separate as arguments.
Add the required import: import { execFileSync } from 'child_process'; if not already present.
Modify all relevant lines within the provided code region.
Suggested changeset
1
e2e/utils/docker.ts
Copilot is powered by AI and may make mistakes. Always verify output.
| dockerUp(); | ||
| } else { | ||
| const restore = `${dockerCompose} exec db pg_restore --clean -U defguard -d defguard /tmp/defguard_backup.dump`; | ||
| execSync(restore); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, we need to avoid passing dynamically constructed shell commands as strings to execSync(), which invokes a shell and interprets special characters. Instead, we should use execFileSync() to run the intended binary (docker), and pass the command’s arguments as an array. This prevents special characters (including spaces) in environment-derived variables from altering the shell command’s behaviour. Specifically, in the block involving the construction and execution of the restore command (line 31), we will refactor the code to:
- Use
execFileSync('docker', [...args])whereargsis an array. - Break out the Docker compose file path, the relevant
exec, service/container, program, and arguments into separate array elements, passing the file path (which may be tainted) as an argument instead of embedding it in a string. - This change should be made only for the affected command on line 31, as per the prompt.
- Import
execFileSync(from 'child_process') if it isn’t already imported, or just use it directly if already present.
Suggested changeset
1
e2e/utils/docker.ts
| @@ -1,4 +1,4 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { execSync, execFileSync } from 'child_process'; | ||
| import path from 'path'; | ||
|
|
||
| const defguardPath = __dirname.split('e2e')[0]; | ||
| @@ -27,8 +27,21 @@ | ||
| if (!dockerCheckContainers()) { | ||
| dockerUp(); | ||
| } else { | ||
| const restore = `${dockerCompose} exec db pg_restore --clean -U defguard -d defguard /tmp/defguard_backup.dump`; | ||
| execSync(restore); | ||
| // Use execFileSync to avoid shell injection from paths or environment values | ||
| execFileSync('docker', [ | ||
| 'compose', | ||
| '-f', | ||
| dockerFilePath, | ||
| 'exec', | ||
| 'db', | ||
| 'pg_restore', | ||
| '--clean', | ||
| '-U', | ||
| 'defguard', | ||
| '-d', | ||
| 'defguard', | ||
| '/tmp/defguard_backup.dump' | ||
| ]); | ||
| const restart = `${dockerCompose} restart db`; | ||
| execSync(restart); | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done'`; |
Copilot is powered by AI and may make mistakes. Always verify output.
| const restore = `${dockerCompose} exec db pg_restore --clean -U defguard -d defguard /tmp/defguard_backup.dump`; | ||
| execSync(restore); | ||
| const restart = `${dockerCompose} restart db`; | ||
| execSync(restart); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix this issue, the shell command should be executed using execFileSync (or spawnSync) from Node's child_process module, providing the command (in this case 'docker') and its arguments as an array. This avoids the use of string interpolation for command construction, preventing any filesystem path with spaces or special shell characters from altering command meaning. Specifically, replace constructions like ${dockerCompose} restart db with direct argument arrays: ['compose', '-f', dockerFilePath, 'restart', 'db'].
Edit only the relevant region in e2e/utils/docker.ts at line 33, ensuring that other usages in the file adhere to this pattern if they might also be susceptible.
You'll need to import execFileSync if not already imported, and replace the call to execSync(restart); with execFileSync('docker', ['compose', '-f', dockerFilePath, 'restart', 'db']);.
Suggested changeset
1
e2e/utils/docker.ts
| @@ -1,4 +1,4 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { execSync, execFileSync } from 'child_process'; | ||
| import path from 'path'; | ||
|
|
||
| const defguardPath = __dirname.split('e2e')[0]; | ||
| @@ -29,8 +29,8 @@ | ||
| } else { | ||
| const restore = `${dockerCompose} exec db pg_restore --clean -U defguard -d defguard /tmp/defguard_backup.dump`; | ||
| execSync(restore); | ||
| const restart = `${dockerCompose} restart db`; | ||
| execSync(restart); | ||
| // Safer restart command, using execFileSync with argument array to prevent shell injection or misparsing paths | ||
| execFileSync('docker', ['compose', '-f', dockerFilePath, 'restart', 'db']); | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done'`; | ||
| execSync(wait_for_db); | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
| const restart = `${dockerCompose} restart db`; | ||
| execSync(restart); | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done'`; | ||
| execSync(wait_for_db); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General approach:
Instead of building up shell commands as concatenated/interpolated strings, pass the executable name and its arguments as separate values to execFileSync (or execSync with {shell: false}). This prevents shell interpretation of spaces or control characters in file paths or environment values.
Detailed fix strategy:
- Anywhere a command is constructed using
${dockerCompose} ..., replace it withexecFileSync('docker', ['compose', '-f', dockerFilePath, ...]). - For subcommands like
exec, pass subcommand/arguments as elements in the argument array. - When using nested shell expressions (e.g., using
sh -c '...'), it's generally safer to split out the arguments as much as possible. If a shell is truly required (e.g., for inline shell loops), escape/interpolate as safely as possible, and minimize the dynamic surface area.
Required changes:
- In
e2e/utils/docker.ts, update eachexecSync(command)and any string built for the command, for all cases (up, exec, restart, etc.). - Import and use
execFileSync(orexecSyncwith argument array and{shell: false}). SinceexecFileSyncis similar and returns a Buffer, it's safe to use here. - Update all references to
dockerComposeand all command construction to remove shell string building and move to argument arrays. - Remove interpolations and avoid concatenations for all dynamic path (or user/environment)-derived data.
Suggested changeset
1
e2e/utils/docker.ts
| @@ -1,25 +1,19 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { execFileSync } from 'child_process'; | ||
| import path from 'path'; | ||
|
|
||
| const defguardPath = __dirname.split('e2e')[0]; | ||
|
|
||
| const dockerFilePath = path.resolve(defguardPath, 'docker-compose.e2e.yaml'); | ||
| const dockerCompose = `docker compose -f ${dockerFilePath}`; | ||
|
|
||
| // Start Defguard stack with docker compose. | ||
| export const dockerUp = () => { | ||
| const command = `${dockerCompose} up --wait`; | ||
| execSync(command); | ||
| runDockerCompose(['up', '--wait']); | ||
| // NOTE: After waiting, sleep for 3 seconds to let Defguard Core apply migrations. | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done; sleep 3'`; | ||
| execSync(wait_for_db); | ||
| const create_snapshot = `${dockerCompose} exec db pg_dump -U defguard -Fc -f /tmp/defguard_backup.dump defguard`; | ||
| execSync(create_snapshot); | ||
| // This requires the shell for the loop/wait, so keep sh -c, but pass shell script as a single argument. | ||
| runDockerCompose(['exec', 'db', 'sh', '-c', "until pg_isready; do sleep 1; done; sleep 3"]); | ||
| runDockerCompose(['exec', 'db', 'pg_dump', '-U', 'defguard', '-Fc', '-f', '/tmp/defguard_backup.dump', 'defguard']); | ||
| }; | ||
|
|
||
| export const dockerCheckContainers = (): boolean => { | ||
| const command = `${dockerCompose} ps -q`; | ||
| const containers = execSync(command).toString().trim(); | ||
| const containers = execFileSync('docker', ['compose', '-f', dockerFilePath, 'ps', '-q']).toString().trim(); | ||
| return Boolean(containers.length); | ||
| }; | ||
|
|
||
| @@ -27,11 +11,8 @@ | ||
| if (!dockerCheckContainers()) { | ||
| dockerUp(); | ||
| } else { | ||
| const restore = `${dockerCompose} exec db pg_restore --clean -U defguard -d defguard /tmp/defguard_backup.dump`; | ||
| execSync(restore); | ||
| const restart = `${dockerCompose} restart db`; | ||
| execSync(restart); | ||
| const wait_for_db = `${dockerCompose} exec db sh -c 'until pg_isready; do sleep 1; done'`; | ||
| execSync(wait_for_db); | ||
| runDockerCompose(['exec', 'db', 'pg_restore', '--clean', '-U', 'defguard', '-d', 'defguard', '/tmp/defguard_backup.dump']); | ||
| runDockerCompose(['restart', 'db']); | ||
| runDockerCompose(['exec', 'db', 'sh', '-c', "until pg_isready; do sleep 1; done"]); | ||
| } | ||
| }; |
Copilot is powered by AI and may make mistakes. Always verify output.
* Merge commit from fork * add redirect URL sanitization * simplify validation * bump outdated dependency * update yanked dependency * bump version to 1.4.1 (#1419) * docs: update enterprise features link (#1337) * Update SECURITY.md * update dependencies * update supported versions --------- Co-authored-by: Saleh Borhani <[email protected]> Co-authored-by: Robert Olejnik <[email protected]>
pre release 1.5 cleanup pt2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge code from a release branch in preparation for 1.5 release