Skip to content

Commit 48c0eed

Browse files
authored
Reapply "Share secrets between Code and Agents app via macOS Keychain" (#313241)
This reverts commit afe5823.
1 parent d943811 commit 48c0eed

18 files changed

Lines changed: 485 additions & 268 deletions

File tree

build/.moduleignore

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ vsda/**
159159
@vscode/policy-watcher/index.d.ts
160160
!@vscode/policy-watcher/build/Release/vscode-policy-watcher.node
161161

162+
@vscode/macos-keychain/build/**
163+
@vscode/macos-keychain/src/**
164+
@vscode/macos-keychain/test/**
165+
@vscode/macos-keychain/binding.gyp
166+
@vscode/macos-keychain/README.md
167+
@vscode/macos-keychain/index.d.ts
168+
!@vscode/macos-keychain/build/Release/keychainNative.node
169+
162170
@vscode/windows-ca-certs/**/*
163171
!@vscode/windows-ca-certs/package.json
164172
!@vscode/windows-ca-certs/**/*.node

build/azure-pipelines/darwin/app-entitlements.plist

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,9 @@
1010
<true/>
1111
<key>com.apple.security.automation.apple-events</key>
1212
<true/>
13+
<key>keychain-access-groups</key>
14+
<array>
15+
<string>$(TeamIdentifierPrefix)com.microsoft.vscode.shared-secrets</string>
16+
</array>
1317
</dict>
1418
</plist>

build/azure-pipelines/darwin/steps/product-build-darwin-compile.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,16 @@ steps:
332332
BUILD_SOURCESDIRECTORY: $(Build.SourcesDirectory)
333333
displayName: ✍️ Codesign & Notarize
334334

335+
# Re-sign the app without the provisioning profile for tests.
336+
# This strips the keychain-access-groups entitlement which requires a
337+
# provisioning profile and is not needed for running tests. The codesign
338+
# step reads from the archives packaged above which have the full entitlements.
339+
- script: |
340+
set -e
341+
export CODESIGN_IDENTITY=$(security find-identity -v -p codesigning $(agent.tempdirectory)/buildagent.keychain | grep -oEi "([0-9A-F]{40})" | head -n 1)
342+
DEBUG=electron-osx-sign* node build/darwin/sign.ts --skip-provisioning-profile $(agent.builddirectory)
343+
displayName: Set Hardened Entitlements (for tests)
344+
335345
- ${{ if or(eq(parameters.VSCODE_RUN_ELECTRON_TESTS, true), eq(parameters.VSCODE_RUN_BROWSER_TESTS, true), eq(parameters.VSCODE_RUN_REMOTE_TESTS, true)) }}:
336346
- template: product-build-darwin-test.yml@self
337347
parameters:

build/darwin/sign.ts

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,14 @@ function getElectronVersion(): string {
1818
return target;
1919
}
2020

21-
function getEntitlementsForFile(filePath: string): string {
21+
const mainProvisioningProfilePath = path.join(baseDir, 'darwin', 'main.provisionprofile');
22+
const agentsProvisioningProfilePath = path.join(baseDir, 'darwin', 'agents.provisionprofile');
23+
24+
function hasProvisioningProfile(): boolean {
25+
return fs.existsSync(mainProvisioningProfilePath);
26+
}
27+
28+
function getEntitlementsForFile(filePath: string, tempDir: string, useProvisioningProfile: boolean, teamId?: string): string {
2229
if (filePath.includes(' Helper (GPU).app')) {
2330
return path.join(baseDir, 'azure-pipelines', 'darwin', 'helper-gpu-entitlements.plist');
2431
} else if (filePath.includes(' Helper (Renderer).app')) {
@@ -28,7 +35,51 @@ function getEntitlementsForFile(filePath: string): string {
2835
} else if (filePath.includes(' Helper.app')) {
2936
return path.join(baseDir, 'azure-pipelines', 'darwin', 'helper-entitlements.plist');
3037
}
31-
return path.join(baseDir, 'azure-pipelines', 'darwin', 'app-entitlements.plist');
38+
const entitlementsPath = path.join(baseDir, 'azure-pipelines', 'darwin', 'app-entitlements.plist');
39+
if (!useProvisioningProfile) {
40+
// Without a provisioning profile, keychain-access-groups entitlement
41+
// will cause signing failures. Strip it from the entitlements plist.
42+
return getStrippedEntitlements(entitlementsPath, tempDir);
43+
}
44+
if (teamId) {
45+
return getExpandedEntitlements(entitlementsPath, tempDir, teamId);
46+
}
47+
return entitlementsPath;
48+
}
49+
50+
let _strippedEntitlementsPath: string | undefined;
51+
52+
/**
53+
* Returns a path to a copy of the entitlements plist with the
54+
* keychain-access-groups key removed.
55+
*/
56+
function getStrippedEntitlements(entitlementsPath: string, tempDir: string): string {
57+
if (!_strippedEntitlementsPath) {
58+
const content = fs.readFileSync(entitlementsPath, 'utf8');
59+
const stripped = content.replace(
60+
/\s*<key>keychain-access-groups<\/key>\s*<array>[\s\S]*?<\/array>/,
61+
''
62+
);
63+
_strippedEntitlementsPath = path.join(tempDir, 'app-entitlements-stripped.plist');
64+
fs.writeFileSync(_strippedEntitlementsPath, stripped);
65+
}
66+
return _strippedEntitlementsPath;
67+
}
68+
69+
let expandedEntitlementsPath: string | undefined;
70+
71+
/**
72+
* Returns a path to a copy of the entitlements plist with
73+
* $(TeamIdentifierPrefix) expanded to the actual team identifier.
74+
*/
75+
function getExpandedEntitlements(entitlementsPath: string, tempDir: string, teamId: string): string {
76+
if (!expandedEntitlementsPath) {
77+
const content = fs.readFileSync(entitlementsPath, 'utf8');
78+
const expanded = content.replace(/\$\(TeamIdentifierPrefix\)/g, teamId + '.');
79+
expandedEntitlementsPath = path.join(tempDir, 'app-entitlements.plist');
80+
fs.writeFileSync(expandedEntitlementsPath, expanded);
81+
}
82+
return expandedEntitlementsPath;
3283
}
3384

3485
async function retrySignOnKeychainError<T>(fn: () => Promise<T>, maxRetries: number = 3): Promise<T> {
@@ -60,7 +111,7 @@ async function retrySignOnKeychainError<T>(fn: () => Promise<T>, maxRetries: num
60111
throw lastError;
61112
}
62113

63-
async function main(buildDir?: string): Promise<void> {
114+
async function main(buildDir?: string, skipProvisioningProfile?: boolean): Promise<void> {
64115
const tempDir = process.env['AGENT_TEMPDIRECTORY'];
65116
const arch = process.env['VSCODE_ARCH'];
66117
const identity = process.env['CODESIGN_IDENTITY'];
@@ -80,23 +131,51 @@ async function main(buildDir?: string): Promise<void> {
80131
? path.resolve(appRoot, appName, 'Contents', 'Applications', `${product.embedded.nameLong}.app`, 'Contents', 'Info.plist')
81132
: undefined;
82133

134+
const useProvisioningProfile = !skipProvisioningProfile && hasProvisioningProfile();
135+
const resolvedProvisioningProfile = useProvisioningProfile ? mainProvisioningProfilePath : undefined;
136+
137+
let teamId: string | undefined;
138+
if (resolvedProvisioningProfile) {
139+
const profilePlist = await spawn('security', ['cms', '-D', '-i', resolvedProvisioningProfile]);
140+
const teamIdMatch = /<key>TeamIdentifier<\/key>\s*<array>\s*<string>(.*?)<\/string>/s.exec(profilePlist);
141+
if (teamIdMatch) {
142+
teamId = teamIdMatch[1];
143+
console.log(`Extracted TeamIdentifier from provisioning profile: ${teamId}`);
144+
} else {
145+
console.warn('Could not extract TeamIdentifier from provisioning profile; $(TeamIdentifierPrefix) will not be expanded');
146+
}
147+
}
148+
149+
// Embed the agents provisioning profile into the embedded app bundle
150+
// before signing, since @electron/osx-sign only supports one top-level profile.
151+
if (useProvisioningProfile && product.embedded && fs.existsSync(agentsProvisioningProfilePath)) {
152+
const embeddedAppPath = path.join(appRoot, appName, 'Contents', 'Applications', `${product.embedded.nameLong}.app`);
153+
if (fs.existsSync(embeddedAppPath)) {
154+
const embeddedProfileDest = path.join(embeddedAppPath, 'Contents', 'embedded.provisionprofile');
155+
fs.copyFileSync(agentsProvisioningProfilePath, embeddedProfileDest);
156+
console.log(`Embedded agents provisioning profile into ${embeddedProfileDest}`);
157+
}
158+
}
159+
83160
const appOpts: SignOptions = {
84161
app: path.join(appRoot, appName),
85162
platform: 'darwin',
86163
optionsForFile: (filePath) => ({
87-
entitlements: getEntitlementsForFile(filePath),
164+
entitlements: getEntitlementsForFile(filePath, tempDir, useProvisioningProfile, teamId),
88165
hardenedRuntime: true,
89166
}),
90167
preAutoEntitlements: false,
91-
preEmbedProvisioningProfile: false,
168+
preEmbedProvisioningProfile: !!resolvedProvisioningProfile,
169+
provisioningProfile: resolvedProvisioningProfile,
92170
keychain: path.join(tempDir, 'buildagent.keychain'),
93171
version: getElectronVersion(),
94172
identity,
95173
};
96174

97175
// Only overwrite plist entries for x64 and arm64 builds,
98176
// universal will get its copy from the x64 build.
99-
if (arch !== 'universal') {
177+
// Skip when re-signing (skipProvisioningProfile) since entries already exist.
178+
if (arch !== 'universal' && !skipProvisioningProfile) {
100179
await spawn('plutil', [
101180
'-insert',
102181
'NSAppleEventsUsageDescription',
@@ -176,7 +255,10 @@ async function main(buildDir?: string): Promise<void> {
176255
}
177256

178257
if (import.meta.main) {
179-
main(process.argv[2]).catch(async err => {
258+
const args = process.argv.slice(2);
259+
const skipProvisioningProfile = args.includes('--skip-provisioning-profile');
260+
const buildDir = args.filter(a => !a.startsWith('--'))[0];
261+
main(buildDir, skipProvisioningProfile).catch(async err => {
180262
console.error(err);
181263
const tempDir = process.env['AGENT_TEMPDIRECTORY'];
182264
if (tempDir) {

package-lock.json

Lines changed: 26 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@
268268
"url": "https://github.com/microsoft/vscode/issues"
269269
},
270270
"optionalDependencies": {
271+
"@vscode/macos-keychain": "^0.0.1",
271272
"windows-foreground-love": "0.6.1"
272273
}
273274
}

product.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"win32TunnelServiceMutex": "vscodeoss-tunnelservice",
2727
"win32TunnelMutex": "vscodeoss-tunnel",
2828
"darwinBundleIdentifier": "com.visualstudio.code.oss",
29+
"darwinSharedKeychainServiceName": "com.visualstudio.code.oss.shared-secrets",
2930
"darwinProfileUUID": "47827DD9-4734-49A0-AF80-7E19B11495CC",
3031
"darwinProfilePayloadUUID": "CF808BE7-53F3-46C6-A7E2-7EDB98A5E959",
3132
"linuxIconName": "code-oss",

src/typings/macos-keychain.d.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
// Type declarations for @vscode/macos-keychain.
7+
// The package is an optional dependency (macOS-only native addon), so types
8+
// are duplicated here to ensure TypeScript compilation succeeds on all platforms.
9+
10+
declare module '@vscode/macos-keychain' {
11+
export function keychainSet(service: string, account: string, value: string): void;
12+
export function keychainGet(service: string, account: string): string | undefined;
13+
export function keychainDelete(service: string, account: string): boolean;
14+
export function keychainList(service: string): string[];
15+
}

src/vs/base/common/product.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ export interface IProductConfiguration {
224224
readonly darwinUniversalAssetId?: string;
225225
readonly darwinBundleIdentifier?: string;
226226
readonly darwinSiblingBundleIdentifier?: string;
227+
readonly darwinSharedKeychainServiceName?: string;
227228
readonly profileTemplatesUrl?: string;
228229

229230
readonly commonlyUsedSettings?: string[];

src/vs/code/electron-main/app.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import { DiagnosticsMainService, IDiagnosticsMainService } from '../../platform/
3737
import { DialogMainService, IDialogMainService } from '../../platform/dialogs/electron-main/dialogMainService.js';
3838
import { IEncryptionMainService } from '../../platform/encryption/common/encryptionService.js';
3939
import { EncryptionMainService } from '../../platform/encryption/electron-main/encryptionMainService.js';
40+
import { ISharedKeychainMainService } from '../../platform/secrets/common/sharedKeychainService.js';
41+
import { SharedKeychainMainService } from '../../platform/secrets/electron-main/sharedKeychainMainService.js';
4042
import { ipcBrowserViewChannelName } from '../../platform/browserView/common/browserView.js';
4143
import { ipcBrowserViewGroupChannelName } from '../../platform/browserView/common/browserViewGroup.js';
4244
import { BrowserViewMainService, IBrowserViewMainService } from '../../platform/browserView/electron-main/browserViewMainService.js';
@@ -1092,6 +1094,9 @@ export class CodeApplication extends Disposable {
10921094
// Encryption
10931095
services.set(IEncryptionMainService, new SyncDescriptor(EncryptionMainService));
10941096

1097+
// Shared Keychain
1098+
services.set(ISharedKeychainMainService, new SyncDescriptor(SharedKeychainMainService));
1099+
10951100
// Cross-app IPC
10961101
services.set(ICrossAppIPCService, new SyncDescriptor(CrossAppIPCService));
10971102

@@ -1270,12 +1275,12 @@ export class CodeApplication extends Disposable {
12701275
this._register(new MacOSCrossAppSecretSharing(
12711276
accessor.get(IStorageMainService),
12721277
accessor.get(IEncryptionMainService),
1278+
accessor.get(ISharedKeychainMainService),
12731279
accessor.get(IStateService),
12741280
this.logService,
12751281
this.environmentMainService,
12761282
accessor.get(ILaunchMainService),
12771283
this.lifecycleMainService,
1278-
crossAppIPCService,
12791284
));
12801285
}
12811286

@@ -1292,6 +1297,10 @@ export class CodeApplication extends Disposable {
12921297
const encryptionChannel = ProxyChannel.fromService(accessor.get(IEncryptionMainService), disposables);
12931298
mainProcessElectronServer.registerChannel('encryption', encryptionChannel);
12941299

1300+
// Shared Keychain
1301+
const sharedKeychainChannel = ProxyChannel.fromService(accessor.get(ISharedKeychainMainService), disposables);
1302+
mainProcessElectronServer.registerChannel('sharedKeychain', sharedKeychainChannel);
1303+
12951304
// Browser View
12961305
const browserViewChannel = ProxyChannel.fromService(accessor.get(IBrowserViewMainService), disposables);
12971306
mainProcessElectronServer.registerChannel(ipcBrowserViewChannelName, browserViewChannel);

0 commit comments

Comments
 (0)