Skip to content

Commit 53f7386

Browse files
brunoborgesCopilot
andcommitted
Address isolated GPG home review feedback
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 9bb9dff commit 53f7386

8 files changed

Lines changed: 97 additions & 17 deletions

File tree

__tests__/auth.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ jest.unstable_mockModule('@actions/core', () => ({
4343

4444
jest.unstable_mockModule('../src/gpg.js', () => ({
4545
importKey: jest.fn(),
46-
removeGpgHome: jest.fn()
46+
removeGpgHome: jest.fn(),
47+
toGpgPath: jest.fn()
4748
}));
4849

4950
// Dynamic imports after mocking
@@ -67,13 +68,15 @@ describe('auth tests', () => {
6768
spyOSHomedir.mockReturnValue(__dirname);
6869
spyInfo = core.info as jest.Mock;
6970
spyInfo.mockImplementation(() => null);
71+
(gpg.toGpgPath as jest.Mock<any>).mockImplementation((p: string) => p);
7072
}, 300000);
7173

7274
afterEach(() => {
7375
(core.getInput as jest.Mock).mockReset();
7476
(core.exportVariable as jest.Mock).mockReset();
7577
(gpg.importKey as jest.Mock).mockReset();
7678
(gpg.removeGpgHome as jest.Mock).mockReset();
79+
(gpg.toGpgPath as jest.Mock).mockReset();
7780
});
7881

7982
afterAll(async () => {
@@ -158,9 +161,11 @@ describe('auth tests', () => {
158161
);
159162
}, 100000);
160163

161-
it('exports and persists the isolated GPG home', async () => {
162-
const gpgHome = path.join(__dirname, 'runner', 'temp', 'setup-java-gpg-1');
164+
it('exports a GPG-compatible path and persists the native GPG home', async () => {
165+
const gpgHome = 'D:\\a\\_temp\\setup-java-gpg-1';
166+
const exportedGpgHome = '/d/a/_temp/setup-java-gpg-1';
163167
(gpg.importKey as jest.Mock<any>).mockResolvedValue(gpgHome);
168+
(gpg.toGpgPath as jest.Mock<any>).mockReturnValue(exportedGpgHome);
164169
(core.getInput as jest.Mock<any>).mockImplementation((name: string) => {
165170
const inputs: Record<string, string> = {
166171
'server-id': 'packages',
@@ -176,7 +181,11 @@ describe('auth tests', () => {
176181

177182
expect(gpg.importKey).toHaveBeenCalledWith('KEY ONE\nKEY TWO');
178183
expect(core.saveState).toHaveBeenCalledWith(STATE_GPG_HOME, gpgHome);
179-
expect(core.exportVariable).toHaveBeenCalledWith('GNUPGHOME', gpgHome);
184+
expect(gpg.toGpgPath).toHaveBeenCalledWith(gpgHome);
185+
expect(core.exportVariable).toHaveBeenCalledWith(
186+
'GNUPGHOME',
187+
exportedGpgHome
188+
);
180189
});
181190

182191
it('removes the isolated GPG home when environment export fails', async () => {

__tests__/gpg.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,28 @@ describe('gpg tests', () => {
188188
await gpg.removeGpgHome(gpgHome);
189189
await gpg.removeGpgHome(gpgHome);
190190

191+
expect(exec.exec).toHaveBeenNthCalledWith(
192+
2,
193+
'gpgconf',
194+
['--homedir', gpg.toGpgPath(gpgHome), '--kill', 'gpg-agent'],
195+
{silent: true, ignoreReturnCode: true}
196+
);
197+
expect(exec.exec).toHaveBeenCalledTimes(2);
191198
expect(fs.existsSync(gpgHome)).toBe(false);
192199
expect(fs.existsSync(unrelatedGpgHome)).toBe(true);
193200
});
194201

202+
it('removes the GPG home when gpgconf is unavailable', async () => {
203+
const gpgHome = await gpg.importKey('KEY CONTENTS');
204+
(exec.exec as jest.Mock<any>).mockRejectedValueOnce(
205+
new Error('gpgconf not found')
206+
);
207+
208+
await gpg.removeGpgHome(gpgHome);
209+
210+
expect(fs.existsSync(gpgHome)).toBe(false);
211+
});
212+
195213
it('refuses to remove a GPG home it does not own', async () => {
196214
const unrelatedGpgHome = path.join(tempDir, 'user-gpg-home');
197215
fs.mkdirSync(unrelatedGpgHome, {recursive: true});

dist/cleanup/index.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35724,7 +35724,7 @@ async function importKey(privateKey) {
3572435724
return gpgHome;
3572535725
}
3572635726
catch (error) {
35727-
await io.rmRF(gpgHome);
35727+
await removeGpgHome(gpgHome);
3572835728
throw error;
3572935729
}
3573035730
}
@@ -35738,6 +35738,15 @@ async function removeGpgHome(gpgHome) {
3573835738
!external_path_.basename(resolvedGpgHome).startsWith(GPG_HOME_PREFIX)) {
3573935739
throw new Error(`Refusing to remove unexpected GPG home: ${gpgHome}`);
3574035740
}
35741+
if (!external_fs_.existsSync(resolvedGpgHome)) {
35742+
return;
35743+
}
35744+
try {
35745+
await lib_exec/* exec */.m('gpgconf', ['--homedir', toGpgPath(resolvedGpgHome), '--kill', 'gpg-agent'], { silent: true, ignoreReturnCode: true });
35746+
}
35747+
catch {
35748+
// gpgconf may be unavailable, but directory removal must still be attempted.
35749+
}
3574135750
await lib_io/* rmRF */.Yz(resolvedGpgHome);
3574235751
}
3574335752
async function verifyPackageSignature(archivePath, signatureUrl, publicKeyContent) {

dist/setup/220.index.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ class MicrosoftDistributions extends base_installer/* JavaBase */.O {
171171
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
172172
/* harmony export */ Fh: () => (/* binding */ importKey),
173173
/* harmony export */ Yi: () => (/* binding */ verifyPackageSignature),
174-
/* harmony export */ mS: () => (/* binding */ removeGpgHome)
174+
/* harmony export */ mS: () => (/* binding */ removeGpgHome),
175+
/* harmony export */ nY: () => (/* binding */ toGpgPath)
175176
/* harmony export */ });
176-
/* unused harmony exports GPG_HOME_PREFIX, toGpgPath */
177+
/* unused harmony export GPG_HOME_PREFIX */
177178
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(9896);
178179
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(fs__WEBPACK_IMPORTED_MODULE_0__);
179180
/* harmony import */ var path__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(6928);
@@ -235,7 +236,7 @@ async function importKey(privateKey) {
235236
return gpgHome;
236237
}
237238
catch (error) {
238-
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(gpgHome);
239+
await removeGpgHome(gpgHome);
239240
throw error;
240241
}
241242
}
@@ -249,6 +250,15 @@ async function removeGpgHome(gpgHome) {
249250
!path__WEBPACK_IMPORTED_MODULE_1__.basename(resolvedGpgHome).startsWith(GPG_HOME_PREFIX)) {
250251
throw new Error(`Refusing to remove unexpected GPG home: ${gpgHome}`);
251252
}
253+
if (!fs__WEBPACK_IMPORTED_MODULE_0__.existsSync(resolvedGpgHome)) {
254+
return;
255+
}
256+
try {
257+
await _actions_exec__WEBPACK_IMPORTED_MODULE_4__/* .exec */ .m('gpgconf', ['--homedir', toGpgPath(resolvedGpgHome), '--kill', 'gpg-agent'], { silent: true, ignoreReturnCode: true });
258+
}
259+
catch {
260+
// gpgconf may be unavailable, but directory removal must still be attempted.
261+
}
252262
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(resolvedGpgHome);
253263
}
254264
async function verifyPackageSignature(archivePath, signatureUrl, publicKeyContent) {

dist/setup/463.index.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ class TemurinDistribution extends base_installer/* JavaBase */.O {
269269
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
270270
/* harmony export */ Fh: () => (/* binding */ importKey),
271271
/* harmony export */ Yi: () => (/* binding */ verifyPackageSignature),
272-
/* harmony export */ mS: () => (/* binding */ removeGpgHome)
272+
/* harmony export */ mS: () => (/* binding */ removeGpgHome),
273+
/* harmony export */ nY: () => (/* binding */ toGpgPath)
273274
/* harmony export */ });
274-
/* unused harmony exports GPG_HOME_PREFIX, toGpgPath */
275+
/* unused harmony export GPG_HOME_PREFIX */
275276
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(9896);
276277
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(fs__WEBPACK_IMPORTED_MODULE_0__);
277278
/* harmony import */ var path__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(6928);
@@ -333,7 +334,7 @@ async function importKey(privateKey) {
333334
return gpgHome;
334335
}
335336
catch (error) {
336-
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(gpgHome);
337+
await removeGpgHome(gpgHome);
337338
throw error;
338339
}
339340
}
@@ -347,6 +348,15 @@ async function removeGpgHome(gpgHome) {
347348
!path__WEBPACK_IMPORTED_MODULE_1__.basename(resolvedGpgHome).startsWith(GPG_HOME_PREFIX)) {
348349
throw new Error(`Refusing to remove unexpected GPG home: ${gpgHome}`);
349350
}
351+
if (!fs__WEBPACK_IMPORTED_MODULE_0__.existsSync(resolvedGpgHome)) {
352+
return;
353+
}
354+
try {
355+
await _actions_exec__WEBPACK_IMPORTED_MODULE_4__/* .exec */ .m('gpgconf', ['--homedir', toGpgPath(resolvedGpgHome), '--kill', 'gpg-agent'], { silent: true, ignoreReturnCode: true });
356+
}
357+
catch {
358+
// gpgconf may be unavailable, but directory removal must still be attempted.
359+
}
350360
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(resolvedGpgHome);
351361
}
352362
async function verifyPackageSignature(archivePath, signatureUrl, publicKeyContent) {

dist/setup/81.index.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ async function configureAuthentication() {
5252
const gpgHome = await _gpg_js__WEBPACK_IMPORTED_MODULE_5__/* .importKey */ .Fh(gpgPrivateKey);
5353
try {
5454
_actions_core__WEBPACK_IMPORTED_MODULE_1__/* .saveState */ .LZ(_constants_js__WEBPACK_IMPORTED_MODULE_7__/* .STATE_GPG_HOME */ .Fi, gpgHome);
55-
_actions_core__WEBPACK_IMPORTED_MODULE_1__/* .exportVariable */ .dN('GNUPGHOME', gpgHome);
55+
_actions_core__WEBPACK_IMPORTED_MODULE_1__/* .exportVariable */ .dN('GNUPGHOME', _gpg_js__WEBPACK_IMPORTED_MODULE_5__/* .toGpgPath */ .nY(gpgHome));
5656
}
5757
catch (error) {
5858
await _gpg_js__WEBPACK_IMPORTED_MODULE_5__/* .removeGpgHome */ .mS(gpgHome);
@@ -132,9 +132,10 @@ async function write(directory, settings, overwriteSettings) {
132132
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
133133
/* harmony export */ Fh: () => (/* binding */ importKey),
134134
/* harmony export */ Yi: () => (/* binding */ verifyPackageSignature),
135-
/* harmony export */ mS: () => (/* binding */ removeGpgHome)
135+
/* harmony export */ mS: () => (/* binding */ removeGpgHome),
136+
/* harmony export */ nY: () => (/* binding */ toGpgPath)
136137
/* harmony export */ });
137-
/* unused harmony exports GPG_HOME_PREFIX, toGpgPath */
138+
/* unused harmony export GPG_HOME_PREFIX */
138139
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(9896);
139140
/* harmony import */ var fs__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(fs__WEBPACK_IMPORTED_MODULE_0__);
140141
/* harmony import */ var path__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(6928);
@@ -196,7 +197,7 @@ async function importKey(privateKey) {
196197
return gpgHome;
197198
}
198199
catch (error) {
199-
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(gpgHome);
200+
await removeGpgHome(gpgHome);
200201
throw error;
201202
}
202203
}
@@ -210,6 +211,15 @@ async function removeGpgHome(gpgHome) {
210211
!path__WEBPACK_IMPORTED_MODULE_1__.basename(resolvedGpgHome).startsWith(GPG_HOME_PREFIX)) {
211212
throw new Error(`Refusing to remove unexpected GPG home: ${gpgHome}`);
212213
}
214+
if (!fs__WEBPACK_IMPORTED_MODULE_0__.existsSync(resolvedGpgHome)) {
215+
return;
216+
}
217+
try {
218+
await _actions_exec__WEBPACK_IMPORTED_MODULE_4__/* .exec */ .m('gpgconf', ['--homedir', toGpgPath(resolvedGpgHome), '--kill', 'gpg-agent'], { silent: true, ignoreReturnCode: true });
219+
}
220+
catch {
221+
// gpgconf may be unavailable, but directory removal must still be attempted.
222+
}
213223
await _actions_io__WEBPACK_IMPORTED_MODULE_3__/* .rmRF */ .Yz(resolvedGpgHome);
214224
}
215225
async function verifyPackageSignature(archivePath, signatureUrl, publicKeyContent) {

src/auth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export async function configureAuthentication() {
5555
const gpgHome = await gpg.importKey(gpgPrivateKey);
5656
try {
5757
core.saveState(constants.STATE_GPG_HOME, gpgHome);
58-
core.exportVariable('GNUPGHOME', gpgHome);
58+
core.exportVariable('GNUPGHOME', gpg.toGpgPath(gpgHome));
5959
} catch (error) {
6060
await gpg.removeGpgHome(gpgHome);
6161
throw error;

src/gpg.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export async function importKey(privateKey: string): Promise<string> {
5858

5959
return gpgHome;
6060
} catch (error) {
61-
await io.rmRF(gpgHome);
61+
await removeGpgHome(gpgHome);
6262
throw error;
6363
}
6464
}
@@ -77,6 +77,20 @@ export async function removeGpgHome(gpgHome: string): Promise<void> {
7777
throw new Error(`Refusing to remove unexpected GPG home: ${gpgHome}`);
7878
}
7979

80+
if (!fs.existsSync(resolvedGpgHome)) {
81+
return;
82+
}
83+
84+
try {
85+
await exec.exec(
86+
'gpgconf',
87+
['--homedir', toGpgPath(resolvedGpgHome), '--kill', 'gpg-agent'],
88+
{silent: true, ignoreReturnCode: true}
89+
);
90+
} catch {
91+
// gpgconf may be unavailable, but directory removal must still be attempted.
92+
}
93+
8094
await io.rmRF(resolvedGpgHome);
8195
}
8296

0 commit comments

Comments
 (0)