Skip to content

Conversation

@pappz
Copy link
Collaborator

@pappz pappz commented Nov 14, 2025

No description provided.

@pappz pappz changed the base branch from main to feature/restart-engine-on-network-change November 14, 2025 23:51
Comment on lines 41 to 61
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
submodules: recursive

- name: Run unit tests
run: ./gradlew test --no-daemon

- name: Upload test results
if: always()
uses: actions/upload-artifact@v4
with:
name: unit-test-results
path: |
app/build/reports/tests/
tool/build/reports/tests/
retention-days: 3

instrumented-tests:

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 15 days ago

The best way to fix this problem is to add a permissions key to the workflow, either at the root or at the job level, specifying the minimal required permissions for the workflow's jobs. In this workflow, none of the jobs perform any action needing write-level permissions on repository contents, issues, or pull requests. Therefore, setting permissions: contents: read at the workflow root will limit the token to read-only repository contents, addressing the security concern in line with GitHub recommendations. This change should be made near the top of .github/workflows/build-debug.yml, right after the name and before the on key for clarity and convention.

Suggested changeset 1
.github/workflows/build-debug.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/build-debug.yml b/.github/workflows/build-debug.yml
--- a/.github/workflows/build-debug.yml
+++ b/.github/workflows/build-debug.yml
@@ -1,4 +1,6 @@
 name: build debug
+permissions:
+  contents: read
 
 on:
   pull_request:
EOF
@@ -1,4 +1,6 @@
name: build debug
permissions:
contents: read

on:
pull_request:
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment on lines 62 to 93
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
submodules: recursive

- name: Enable KVM group perms
run: |
echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules
sudo udevadm control --reload-rules
sudo udevadm trigger --name-match=kvm
- name: Run instrumented tests
uses: reactivecircus/android-emulator-runner@v2
with:
api-level: 30
target: google_apis
arch: x86_64
profile: pixel_3a
disable-animations: true
script: ./gradlew connectedDebugAndroidTest --no-daemon

- name: Upload test results
if: always()
uses: actions/upload-artifact@v4
with:
name: instrumented-test-results
path: |
app/build/reports/androidTests/
tool/build/reports/androidTests/
retention-days: 3

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 15 days ago

To fix this issue, you should add a permissions: block at either the workflow root or in each job entry. The recommended fix is to add it at the root of the workflow, which will apply to all jobs unless a more specific permissions: block is set inside a job. Since none of the jobs in the workflow require write access (they only check out code, run builds/tests, and upload artifacts), the minimal required permission is contents: read. This is standard and will adhere to the principle of least privilege. There is no need to change or add any other functionality; the fix is a single line addition near the top, after the name: and before on:.

Suggested changeset 1
.github/workflows/build-debug.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/build-debug.yml b/.github/workflows/build-debug.yml
--- a/.github/workflows/build-debug.yml
+++ b/.github/workflows/build-debug.yml
@@ -1,4 +1,6 @@
 name: build debug
+permissions:
+  contents: read
 
 on:
   pull_request:
EOF
@@ -1,4 +1,6 @@
name: build debug
permissions:
contents: read

on:
pull_request:
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
@doromaraujo doromaraujo self-assigned this Nov 17, 2025
@doromaraujo
Copy link
Collaborator

Approved. Feel free to merge it.

@pappz
Copy link
Collaborator Author

pappz commented Nov 18, 2025

.

Could you approve in github too? https://github.com/netbirdio/android-client/pull/99/files and click on the submit review -> approve

@pappz pappz requested a review from doromaraujo November 18, 2025 11:43
Copy link
Collaborator

@doromaraujo doromaraujo left a comment

Choose a reason for hiding this comment

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

Approved.

@pappz pappz merged commit e4a47e7 into feature/restart-engine-on-network-change Nov 18, 2025
3 checks passed
doromaraujo added a commit that referenced this pull request Nov 18, 2025
* Add network change detector and listeners

* Add network toggle listener to VPNService

* Add unit tests for ConcreteNetworkChangeListener

* Rename NetworkChangeListener to NetworkAvailabilityListener

* Change EngineRunner's Set implementation

From HashSet to ConcurrentHashMap's KeySet,
which is thread-safe.

* Update submodule to the latest tag (v0.59.6)

* Add EngineRestarter to restart the Go engine

* Add LOGTAG to NetworkChangeDetector

* Add some documentation to NetworkToggleListener

* Use EngineRestarter as implementation of NetworkToggleListener

When subscribing to ConcreteNetworkAvailabilityListener

* Replace HashMap usage with ConcurrentHashMap

The availableNetworkTypes HashMap is accessed from network
callback threads without synchronization

* Wrap connectivityManager.unregisterNetworkCallback with try-catch

* Add restart runnable and timeout callback to EngineRestarter

Restart runnable is used as a debouncing mechanism to
prevent concurrent restarts
Timeout callback is to reset the isRestartInProgress flag's
value if the engine takes too long to restart

* Update git submodule reference to latest tag

* Reverse cleanup order on VPNService's onDestroy

Now it disposes of network listener components
before stopping engineRunner

* Ci tests (#99)

Add testing steps for CI

* Remove line break

---------

Co-authored-by: Zoltan Papp <zoltan.pmail@gmail.com>
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.

3 participants