Re-land Create SPI for Enhanced Security#51115
Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom Sep 24, 2025
Merged
Re-land Create SPI for Enhanced Security#51115webkit-commit-queue merged 1 commit intoWebKit:mainfrom
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Conversation
Collaborator
|
EWS run on previous version of this PR (hash e1d21ca) Details
|
darryl-apple
added
the
New Bugs
darryl-apple
changed the title
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
e1d21ca to
2fd43bb
Compare
Collaborator
|
EWS run on previous version of this PR (hash 2fd43bb) Details |
🛠 mac-apple
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
2fd43bb to
b1829f9
Compare
Collaborator
|
EWS run on previous version of this PR (hash b1829f9) Details |
weinig
requested changes
Sep 22, 2025
Source/WebCore/testing/Internals.h
Outdated
| @@ -1035,6 +1035,8 @@ class Internals final | |||
|
|
|||
| bool isHardwareVP9DecoderExpected(); | |||
|
|
|||
| bool enhancedSecurityEnabled() const; | |||
Contributor
There was a problem hiding this comment.
As I mentioned in another PR, while I think having the term "enhanced security" in Apple's WebKit API/SPI is fine, I really don't think it belongs in the internals as it is not a term that is meaningful or has standards associated with it.
Contributor
Author
There was a problem hiding this comment.
Hey - Thanks for the feedback! We'll look at breaking it down into more specifics as the feature develops
Contributor
There was a problem hiding this comment.
Can you go into more detail about why WebCore internals need to know about this state at the moment?
Contributor
There was a problem hiding this comment.
This is purely to expose to LayoutTests when we are/aren't running in a given configuration. We actually want the same thing for LockdownMode too.
How about an alternate proposal of DOMString webContentProcessVariant() which returns 'standard', 'lockdown', 'security', etc.? That way, we can have a generic routine which can be re-used throughout the various testing scenarios we would like to cover.
Contributor
There was a problem hiding this comment.
Those seem like things that are probably better tested through API tests.
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
b1829f9 to
5705a2f
Compare
Collaborator
|
EWS run on previous version of this PR (hash 5705a2f) Details |
webkit-ews-buildbot
added
the
merging-blocked
pvollan
reviewed
Sep 23, 2025
Comment on lines
+6563
to
+6580
| String Internals::webContentProcessVariant() const | ||
| { | ||
| auto* document = contextDocument(); | ||
| if (!document) | ||
| return { }; | ||
| auto* page = document->page(); | ||
| if (!page) | ||
| return { }; | ||
|
|
||
| switch (page->webContentProcessVariant()) { | ||
| case WebContentProcessVariant::Security: | ||
| return "security"_s; | ||
| case WebContentProcessVariant::Lockdown: | ||
| return "lockdown"_s; | ||
| default: | ||
| return "standard"_s; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
I wonder if we should use entitlements checks instead to determine the WebContent process variant. What do you think?
Contributor
There was a problem hiding this comment.
I agree that we should do this and is the intent. But this PR doesn’t have the code to spin the new process type up - that’s in the follow-up. We didn’t want to merge the 2 PRs and it be too large to review. The hope was that this small amount of glue code would be enough to get an initial set of tests passing then in the follow-up we’d replace it with a process check using entitlement/process name or similar.
Contributor
There was a problem hiding this comment.
Sounds good, thanks! Should we file a bug, and add a comment about this? I think we can move forward with the current approach, if Sam does not have concerns with the Internals changes.
Contributor
There was a problem hiding this comment.
WebCore, and therefore this Internals class, really should not know anything about web process, as that is a WebKit concept.
I do think this would be better as an API test.
Contributor
There was a problem hiding this comment.
That's a good point @weinig! @darryl-apple, would you have time to create API tests for this instead?
darryl-apple
removed
the
merging-blocked
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
5705a2f to
7419202
Compare
Collaborator
|
EWS run on previous version of this PR (hash 7419202) Details
|
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
7419202 to
082c839
Compare
Collaborator
|
EWS run on previous version of this PR (hash 082c839) Details
|
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
082c839 to
d0ad450
Compare
Collaborator
|
EWS run on previous version of this PR (hash d0ad450) Details |
pvollan
reviewed
Sep 23, 2025
Source/WebCore/page/Page.h
Outdated
| @@ -1380,6 +1386,9 @@ class Page : public RefCountedAndCanMakeWeakPtr<Page>, public Supplementable<Pag | |||
| void setHardwareKeyboardAttached(bool attached) { m_hardwareKeyboardAttached = attached; } | |||
| bool hardwareKeyboardAttached() const { return m_hardwareKeyboardAttached; } | |||
|
|
|||
| void setWebContentProcessVariant(WebContentProcessVariant value) { m_webContentProcessVariant = value; }; | |||
pvollan
reviewed
Sep 23, 2025
| @@ -12227,6 +12229,12 @@ void WebPageProxy::isJITEnabled(CompletionHandler<void(bool)>&& completionHandle | |||
| protectedLegacyMainFrameProcess()->sendWithAsyncReply(Messages::WebProcess::IsJITEnabled(), WTFMove(completionHandler), 0); | |||
| } | |||
|
|
|||
| void WebPageProxy::isEnhancedSecurityEnabled(CompletionHandler<void(bool)>&& completionHandler) | |||
| { | |||
| launchInitialProcessIfNecessary(); | |||
Contributor
There was a problem hiding this comment.
Should we just call the completion handler with false if a WebContent process hasn't been launched yet?
Contributor
Author
There was a problem hiding this comment.
Yes that seems sensible. Changed
pvollan
reviewed
Sep 23, 2025
| @@ -145,6 +145,7 @@ messages -> WebProcess : AuxiliaryProcess WantsAsyncDispatchMessage { | |||
| #endif | |||
|
|
|||
| IsJITEnabled() -> (bool enabled) | |||
| IsEnhancedSecurityEnabled() -> (bool enabled) | |||
Contributor
There was a problem hiding this comment.
We may replace this with an entitlement check at a later point.
darryl-apple
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
d0ad450 to
6ac8c24
Compare
Collaborator
|
EWS run on current version of this PR (hash 6ac8c24) Details |
pvollan
approved these changes
Sep 23, 2025
darryl-apple
added
the
safe-merge-queue
webkit-ews-buildbot
added
merge-queue
Collaborator
|
Safe-Merge-Queue: Build #70176. |
https://bugs.webkit.org/show_bug.cgi?id=299285 rdar://161090983 Reviewed by Per Arne Vollan. Create SPI for EnhancedSecurity Tests: security/cocoa/check-enhanced-security-disabled.html security/cocoa/check-enhanced-security-enabled.html security/cocoa/enhanced-security/check-enhanced-security-enabled.html security/cocoa/lockdown-mode/check-lockdown-mode-enabled.html Tools/TestWebKitAPI/SourcesCocoa.txt Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj Tools/TestWebKitAPI/Tests/WebKitCocoa/EnhancedSecurity.mm * LayoutTests/TestExpectations: * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/mac-wk2/TestExpectations: * LayoutTests/security/cocoa/check-enhanced-security-disabled-expected.txt: Added. * LayoutTests/security/cocoa/check-enhanced-security-disabled.html: Added. * LayoutTests/security/cocoa/check-enhanced-security-enabled-expected.txt: Added. * LayoutTests/security/cocoa/check-enhanced-security-enabled.html: Added. * LayoutTests/security/cocoa/enhanced-security/check-enhanced-security-enabled-expected.txt: Added. * LayoutTests/security/cocoa/enhanced-security/check-enhanced-security-enabled.html: Added. * LayoutTests/security/cocoa/lockdown-mode/check-lockdown-mode-enabled-expected.txt: Added. * LayoutTests/security/cocoa/lockdown-mode/check-lockdown-mode-enabled.html: Added. * Source/WebCore/page/Page.h: (WebCore::Page::setWebContentProcessVariant): (WebCore::Page::webContentProcessVariant const): * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::webContentProcessVariant const): * Source/WebCore/testing/Internals.h: * Source/WebCore/testing/Internals.idl: * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceEntryPoint.h: (WebKit::XPCServiceInitializer): * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceEntryPoint.mm: (WebKit::XPCServiceInitializerDelegate::getExtraInitializationData): (WebKit::setJSCOptions): * Source/WebKit/UIProcess/API/APIPageConfiguration.cpp: (API::PageConfiguration::enhancedSecurityEnabled const): * Source/WebKit/UIProcess/API/APIPageConfiguration.h: * Source/WebKit/UIProcess/API/APIWebsitePolicies.cpp: (API::WebsitePolicies::copy const): * Source/WebKit/UIProcess/API/APIWebsitePolicies.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _isEnhancedSecurityEnabled:]): * Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm: (-[WKWebpagePreferences _setEnhancedSecurityEnabled:]): (-[WKWebpagePreferences _enhancedSecurityEnabled]): * Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferencesPrivate.h: * Source/WebKit/UIProcess/BrowsingContextGroup.cpp: (WebKit::BrowsingContextGroup::sharedProcessForSite): * Source/WebKit/UIProcess/BrowsingContextGroup.h: * Source/WebKit/UIProcess/SuspendedPageProxy.cpp: (WebKit::SuspendedPageProxy::findReusableSuspendedPageProcess): * Source/WebKit/UIProcess/SuspendedPageProxy.h: * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::launchProcess): (WebKit::WebPageProxy::receivedNavigationActionPolicyDecision): (WebKit::WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation): (WebKit::WebPageProxy::isEnhancedSecurityEnabled): (WebKit::WebPageProxy::shouldEnableEnhancedSecurity const): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebProcessCache.cpp: (WebKit::WebProcessCache::takeProcess): * Source/WebKit/UIProcess/WebProcessCache.h: * Source/WebKit/UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::establishRemoteWorkerContextConnectionToNetworkProcess): (WebKit::WebProcessPool::createNewWebProcess): (WebKit::WebProcessPool::tryTakePrewarmedProcess): (WebKit::WebProcessPool::prewarmProcess): (WebKit::WebProcessPool::processForSite): (WebKit::WebProcessPool::createWebPage): (WebKit::WebProcessPool::processForNavigation): (WebKit::WebProcessPool::prepareProcessForNavigation): (WebKit::WebProcessPool::processForNavigationInternal): * Source/WebKit/UIProcess/WebProcessPool.h: * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::create): (WebKit::WebProcessProxy::createForRemoteWorkers): (WebKit::WebProcessProxy::WebProcessProxy): (WebKit::WebProcessProxy::getLaunchOptions): * Source/WebKit/UIProcess/WebProcessProxy.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::m_toolbarsAreVisible): * Source/WebKit/WebProcess/WebProcess.cpp: (WebKit::WebProcess::initializeProcess): (WebKit::WebProcess::isEnhancedSecurityEnabled): * Source/WebKit/WebProcess/WebProcess.h: * Source/WebKit/WebProcess/WebProcess.messages.in: * Tools/MiniBrowser/mac/SettingsController.h: * Tools/MiniBrowser/mac/SettingsController.m: (-[SettingsController _populateMenu:]): (-[SettingsController validateMenuItem:]): (-[SettingsController toggleEnhancedSecurityEnabled:]): (-[SettingsController enhancedSecurityEnabled]): * Tools/MiniBrowser/mac/WK2BrowserWindowController.m: (-[WK2BrowserWindowController didChangeSettings]): * Tools/TestRunnerShared/TestFeatures.cpp: (WTR::shouldEnableEnhancedSecurity): (WTR::hardcodedFeaturesBasedOnPathForTest): * Tools/TestWebKitAPI/SourcesCocoa.txt: * Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: * Tools/TestWebKitAPI/Tests/WebKitCocoa/EnhancedSecurity.mm: Added. (TestWebKitAPI::isEnhancedSecurityEnabled): (TestWebKitAPI::isJITEnabled): (TestWebKitAPI::TEST(EnhancedSecurity, EnhancedSecurityEnablesTrue)): (TestWebKitAPI::TEST(EnhancedSecurity, EnhancedSecurityEnableFalse)): (TestWebKitAPI::TEST(EnhancedSecurity, EnhancedSecurityDisablesJIT)): (TestWebKitAPI::TEST(EnhancedSecurity, EnhancedSecurityNavigationStaysEnabledAfterNavigation)): (TestWebKitAPI::TEST(EnhancedSecurity, PSONToEnhancedSecurity)): (TestWebKitAPI::TEST(EnhancedSecurity, PSONToEnhancedSecuritySamePage)): (TestWebKitAPI::psonProcessPoolConfiguration): (TestWebKitAPI::TEST(EnhancedSecurity, PSONToEnhancedSecuritySharedProcessPool)): (TestWebKitAPI::TEST(EnhancedSecurity, PSONToEnhancedSecuritySharedProcessPoolReverse)): * Tools/WebKitTestRunner/InjectedBundle/TestRunner.h: * Tools/WebKitTestRunner/TestOptions.cpp: (WTR::TestOptions::defaults): (WTR::TestOptions::keyTypeMapping): * Tools/WebKitTestRunner/TestOptions.h: (WTR::TestOptions::enhancedSecurityEnabled const): * Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm: (WTR::TestController::configureWebpagePreferences): Canonical link: https://commits.webkit.org/300457@main
webkit-commit-queue
force-pushed
the
eng/Reland-Create-WKAPI-for-Enhanced-Security
branch
from
6ac8c24 to
347f872
Compare
Collaborator
|
Committed 300457@main (347f872): https://commits.webkit.org/300457@main Reviewed commits have been landed. Closing PR #51115 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
347f872
6ac8c24