-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Safari support #125
base: develop
Are you sure you want to change the base?
feat: Safari support #125
Changes from 135 commits
9141351
ac9dac0
feb531d
64f5b77
36af976
ee4c32a
65e5326
92f85d2
ac123c1
bfce9c6
1f62d71
3a86dd6
93806c9
10f6316
f7d1056
e6c814d
d5b5580
d5425a3
9f73e86
00c3e5d
08779be
c28db05
6a6953a
5a05aa9
8e08fb6
e4a69d0
108f509
064c49b
60e03e6
d6f9265
17b1de5
3fb1ff2
f5f3175
e25a53f
2a5de5a
b6129e4
266ff38
e1dd474
b04d996
af074f1
39f3e48
647b64c
190a742
4d8bf3e
3ae790d
82115be
44249fe
d3de924
df6e41f
bf3d72c
7e58477
e5a8d8e
0b0e44e
cbce89c
7c35165
470931e
ec14d93
df492dd
dff4aa5
f91c4ae
7ea0ad3
929cf08
c318ec5
dce77fe
4471a33
e2c7089
30378ff
cc05788
db9dba3
52699d7
1a1fb18
2589de7
baa4d4f
7e828b1
6217a45
941974d
09a806d
e3c213b
21aeee3
8b84b74
45f11b3
c5bdd2a
4d15efc
ee9368b
0fe7b3b
a6c9634
59866de
e916c0e
a2df125
137aeb2
5676d73
fa07d7a
1ef62ce
673046b
538e461
6f3c791
13deed1
5a19af4
07b7f0f
7188794
1ce568c
bd7fc06
dbca981
0ee90eb
dd01c75
9f60b6c
8b2b540
5d47410
3c3310e
38ad2b3
5b847ff
127f32c
d18970e
7570172
4603ad5
e479489
bc874a6
bf54bdd
9081e97
01a0317
f4e2df4
23c9f74
3c55268
e8b650c
21034fc
d1bd5a5
221a3ef
c49dc6d
4a762a1
6e52981
f78205f
7fde905
1db5c44
97cae1f
416e344
67a01bf
bc85ab7
3a5e193
7700199
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,92 @@ | ||||||||||||||||||||||
name: MacOS Safari Deploy | ||||||||||||||||||||||
|
||||||||||||||||||||||
on: [push] | ||||||||||||||||||||||
|
||||||||||||||||||||||
jobs: | ||||||||||||||||||||||
buildApp: | ||||||||||||||||||||||
name: safari build | ||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||
steps: | ||||||||||||||||||||||
- uses: actions/checkout@v3 | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: setup env | ||||||||||||||||||||||
id: node-version | ||||||||||||||||||||||
run: | | ||||||||||||||||||||||
docker build -t enkrypt-build-container . | ||||||||||||||||||||||
echo "NODE_VERSION=$(docker run --rm -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "node --version")" >> $GITHUB_OUTPUT | ||||||||||||||||||||||
Comment on lines
+14
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix shell script safety issues The shell commands have several safety issues that could cause failures in edge cases. Apply these fixes: - docker build -t enkrypt-build-container .
- echo "NODE_VERSION=$(docker run --rm -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "node --version")" >> $GITHUB_OUTPUT
+ docker build -t enkrypt-build-container .
+ echo "NODE_VERSION=$(docker run --rm -v "$(pwd)":/home:rw enkrypt-build-container /bin/bash -c "node --version")" >> $GITHUB_OUTPUT
- run: docker run --rm -v `pwd`:/home:rw --user "$(id -u):$(id -g)" enkrypt-build-container /bin/bash -c "yarn install --silent"
+ run: docker run --rm -v "$(pwd)":/home:rw --user "$(id -u):$(id -g)" enkrypt-build-container /bin/bash -c "yarn install --silent"
- docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:all && yarn build:safari"
+ docker run --rm --user "$(id -u):$(id -g)" -v "$(pwd)":/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:all && yarn build:safari" Also applies to: 30-30, 34-35 🧰 Tools🪛 actionlint (1.7.4)14-14: shellcheck reported issue in this script: SC2046:warning:2:41: Quote this to prevent word splitting (shellcheck) 14-14: shellcheck reported issue in this script: SC2006:style:2:41: Use $(...) notation instead of legacy backticks (shellcheck) 14-14: shellcheck reported issue in this script: SC2086:info:2:115: Double quote to prevent globbing and word splitting (shellcheck) |
||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Get yarn cache directory path | ||||||||||||||||||||||
id: yarn-cache-dir-path | ||||||||||||||||||||||
run: echo "dir=$(yarn config get cacheFolder)" >> $GITHUB_OUTPUT | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Cache yarn modules | ||||||||||||||||||||||
uses: actions/cache@v3 | ||||||||||||||||||||||
id: yarn-cache | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
path: ${{ steps.yarn-cache-dir-path.outputs.dir }} | ||||||||||||||||||||||
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: yarn install | ||||||||||||||||||||||
run: docker run --rm -v `pwd`:/home:rw --user "$(id -u):$(id -g)" enkrypt-build-container /bin/bash -c "yarn install --silent" | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: build | ||||||||||||||||||||||
run: | | ||||||||||||||||||||||
mkdir release | ||||||||||||||||||||||
docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:all && yarn build:safari" | ||||||||||||||||||||||
|
||||||||||||||||||||||
- uses: actions/upload-artifact@v3 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
name: safari-build-artifact | ||||||||||||||||||||||
path: packages/extension/dist | ||||||||||||||||||||||
retention-days: 1 | ||||||||||||||||||||||
|
||||||||||||||||||||||
deploy: | ||||||||||||||||||||||
name: Deploying to appstore | ||||||||||||||||||||||
needs: buildApp | ||||||||||||||||||||||
runs-on: macOS-latest | ||||||||||||||||||||||
|
||||||||||||||||||||||
steps: | ||||||||||||||||||||||
- uses: maxim-lobanov/setup-xcode@v1 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
xcode-version: "latest-stable" | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Checkout repository | ||||||||||||||||||||||
uses: actions/checkout@v3 | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Retrieve saved safari build | ||||||||||||||||||||||
uses: actions/download-artifact@v3 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
name: safari-build-artifact | ||||||||||||||||||||||
path: packages/extension/dist | ||||||||||||||||||||||
- uses: ruby/setup-ruby@v1 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
ruby-version: "3.0" | ||||||||||||||||||||||
bundler-cache: true | ||||||||||||||||||||||
working-directory: "macos/Enkrypt" | ||||||||||||||||||||||
- name: Deploy via Fastlane | ||||||||||||||||||||||
uses: maierj/[email protected] | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
lane: "buildandrelease" | ||||||||||||||||||||||
subdirectory: "macos/Enkrypt" | ||||||||||||||||||||||
env: | ||||||||||||||||||||||
APPLE_ID: "${{ secrets.OSX_APPLE_ID }}" | ||||||||||||||||||||||
APP_ID: "${{ secrets.OSX_APP_ID }}" | ||||||||||||||||||||||
FASTLANE_ITC_TEAM_ID: "${{ secrets.OSX_FASTLANE_ITC_TEAM_ID }}" | ||||||||||||||||||||||
FASTLANE_APPLE_APPLICATION_SPECIFIC_PASSWORD: "${{ secrets.OSX_FASTLANE_APPLE_APPLICATION_SPECIFIC_PASSWORD }}" | ||||||||||||||||||||||
ITC_TEAM_ID: "${{ secrets.OSX_FASTLANE_ITC_TEAM_ID }}" | ||||||||||||||||||||||
INSTALLER_CERT: "${{ secrets.OSX_INSTALLER_CERT }}" | ||||||||||||||||||||||
SAFARI_PROVISION: "${{ secrets.OSX_SAFARI_PROVISION }}" | ||||||||||||||||||||||
EXTENSION_PROVISION: "${{ secrets.OSX_EXTENSION_PROVISION }}" | ||||||||||||||||||||||
MATCH_GIT_URL: "${{ secrets.OSX_MATCH_GIT_URL }}" | ||||||||||||||||||||||
MATCH_PASSWORD: "${{ secrets.OSX_MATCH_PASSWORD }}" | ||||||||||||||||||||||
GIT_AUTHORIZATION: "${{ secrets.OSX_GIT_AUTHORIZATION }}" | ||||||||||||||||||||||
API_KEY: "${{ secrets.OSX_API_KEY }}" | ||||||||||||||||||||||
API_KEY_ID: "${{ secrets.OSX_API_KEY_ID }}" | ||||||||||||||||||||||
API_ISSUER_ID: "${{ secrets.OSX_API_ISSUER_ID }}" | ||||||||||||||||||||||
|
||||||||||||||||||||||
Comment on lines
+66
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for deployment steps The deployment steps lack explicit error handling which could make debugging failures difficult. Consider adding:
Example: - name: Deploy via Fastlane
uses: maierj/[email protected]
with:
lane: "buildandrelease"
subdirectory: "macos/Enkrypt"
+ timeout-minutes: 30
+ continue-on-error: false
env:
APPLE_ID: "${{ secrets.OSX_APPLE_ID }}"
# ... other secrets ...
+ - name: Upload deployment logs on failure
+ if: failure()
+ uses: actions/upload-artifact@v4
+ with:
+ name: deployment-logs
+ path: macos/Enkrypt/fastlane/logs
+ retention-days: 5
|
||||||||||||||||||||||
- name: Upload Enkrypt pkg | ||||||||||||||||||||||
uses: actions/upload-artifact@v3 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
name: safari-build-pkg | ||||||||||||||||||||||
path: macos/Enkrypt/build/Enkrypt.pkg | ||||||||||||||||||||||
retention-days: 1 | ||||||||||||||||||||||
Comment on lines
+88
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update outdated GitHub Action The - uses: actions/upload-artifact@v3
+ uses: actions/upload-artifact@v4 📝 Committable suggestion
Suggested change
🧰 Tools🪛 actionlint (1.7.4)88-88: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue (action) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ on: | |
- "v*" | ||
jobs: | ||
release: | ||
name: new release | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add Safari build step to release workflow The workflow is missing steps to build and package the Safari extension. This is necessary for the Safari support feature. Add Safari build step after the Firefox build: - name: build
env:
VIRUS_TOTAL_API_KEY: ${{secrets.VIRUS_TOTAL_API_KEY}}
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
run: |
mkdir release
docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "yarn build:all"
docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:chrome && yarn zip"
mv packages/extension/dist/release.zip release/enkrypt-chrome-edge-opera-${{ steps.get_release_tag.outputs.VERSION }}.zip
docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:firefox && yarn zip"
mv packages/extension/dist/release.zip release/enkrypt-firefox-${{ steps.get_release_tag.outputs.VERSION }}.xpi
+ docker run --rm --user "$(id -u):$(id -g)" -v `pwd`:/home:rw enkrypt-build-container /bin/bash -c "cd packages/extension && yarn build:safari && yarn zip"
+ mv packages/extension/dist/release.zip release/enkrypt-safari-${{ steps.get_release_tag.outputs.VERSION }}.zip
|
||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ on: [push] | |
|
||
jobs: | ||
test: | ||
name: test all | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Mac OS X | ||
*.DS_Store | ||
|
||
# Xcode | ||
*.pbxuser | ||
*.mode1v3 | ||
*.mode2v3 | ||
*.perspectivev3 | ||
*.xcuserstate | ||
project.xcworkspace/ | ||
xcuserdata/ | ||
|
||
# Generated files | ||
*.o | ||
*.pyc | ||
|
||
|
||
#Python modules | ||
MANIFEST | ||
dist/ | ||
build/ | ||
|
||
# Backup files | ||
*~.nib |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
<plist version="1.0"> | ||
<dict> | ||
<key>com.apple.security.app-sandbox</key> | ||
<true/> | ||
<key>com.apple.security.files.user-selected.read-only</key> | ||
<true/> | ||
</dict> | ||
</plist> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<?xml version="1.0" encoding="UTF-8"?> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<plist version="1.0"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<dict> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<key>NSExtension</key> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<dict> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<key>NSExtensionPointIdentifier</key> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<string>com.apple.Safari.web-extension</string> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<key>NSExtensionPrincipalClass</key> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<string>$(PRODUCT_MODULE_NAME).SafariWebExtensionHandler</string> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</dict> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</dict> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add recommended Info.plist keys for Safari extension The current Info.plist is missing several recommended keys for Safari extensions. Consider adding the following: <dict>
+ <key>CFBundleIdentifier</key>
+ <string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
+ <key>CFBundleVersion</key>
+ <string>1</string>
+ <key>CFBundleShortVersionString</key>
+ <string>1.0</string>
+ <key>CFBundleDisplayName</key>
+ <string>Enkrypt</string>
+ <key>Description</key>
+ <string>Enkrypt Safari Extension</string>
<key>NSExtension</key>
<dict>
<key>NSExtensionPointIdentifier</key> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</plist> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||
// SafariWebExtensionHandler.swift | ||||||||||||||||||||||||||
// Enkrypt Extension | ||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||
// Created by admin on 8/16/22. | ||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import SafariServices | ||||||||||||||||||||||||||
import os.log | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
let SFExtensionMessageKey = "message" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling { | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
func beginRequest(with context: NSExtensionContext) { | ||||||||||||||||||||||||||
let item = context.inputItems[0] as! NSExtensionItem | ||||||||||||||||||||||||||
let message = item.userInfo?[SFExtensionMessageKey] | ||||||||||||||||||||||||||
os_log(.default, "Received message from browser.runtime.sendNativeMessage: %@", message as! CVarArg) | ||||||||||||||||||||||||||
Comment on lines
+15
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove force unwrapping to prevent crashes The current implementation uses force unwrapping ( - func beginRequest(with context: NSExtensionContext) {
- let item = context.inputItems[0] as! NSExtensionItem
- let message = item.userInfo?[SFExtensionMessageKey]
- os_log(.default, "Received message from browser.runtime.sendNativeMessage: %@", message as! CVarArg)
+ func beginRequest(with context: NSExtensionContext) {
+ guard let item = context.inputItems.first as? NSExtensionItem,
+ let message = item.userInfo?[SFExtensionMessageKey] else {
+ os_log(.error, "Failed to process extension request: invalid input")
+ context.completeRequest(returningItems: nil, completionHandler: nil)
+ return
+ }
+ os_log(.default, "Received message from browser.runtime.sendNativeMessage: %{public}@", String(describing: message)) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
let response = NSExtensionItem() | ||||||||||||||||||||||||||
response.userInfo = [ SFExtensionMessageKey: [ "Response to": message ] ] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
context.completeRequest(returningItems: [response], completionHandler: nil) | ||||||||||||||||||||||||||
Comment on lines
+20
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add structured error handling and response validation The response handling could be more robust with proper error handling and response validation. - let response = NSExtensionItem()
- response.userInfo = [ SFExtensionMessageKey: [ "Response to": message ] ]
-
- context.completeRequest(returningItems: [response], completionHandler: nil)
+ do {
+ let response = NSExtensionItem()
+ let responseMessage = [ "Response to": message ]
+ response.userInfo = [ SFExtensionMessageKey: responseMessage ]
+
+ context.completeRequest(returningItems: [response]) { error in
+ if let error = error {
+ os_log(.error, "Failed to complete request: %{public}@", error.localizedDescription)
+ }
+ }
+ } catch {
+ os_log(.error, "Failed to process response: %{public}@", error.localizedDescription)
+ context.completeRequest(returningItems: nil, completionHandler: nil)
+ }
|
||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict workflow trigger to relevant events
Running the deployment workflow on every push could lead to unnecessary builds and potential App Store submission rate limits.
Consider limiting to specific branches or tags:
📝 Committable suggestion