Skip to content
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

Add set-safe-directory input to allow customer to take control. #770

Merged
merged 3 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,41 @@ jobs:
path: basic
- name: Verify basic
run: __test__/verify-basic.sh --archive

test-git-container:
runs-on: ubuntu-latest
container: bitnami/git:latest
Copy link
Member Author

Choose a reason for hiding this comment

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

we will have e2e coverage on any newer git version release.

Choose a reason for hiding this comment

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

มันน่าจะดีมากขึ้นไปอีก ผมเคารพการตัดสินใจของชุมชน

steps:
# Clone this repo
- name: Checkout
uses: actions/checkout@v3
with:
path: v3

# Basic checkout using git
- name: Checkout basic
uses: ./v3
with:
ref: test-data/v2/basic
- name: Verify basic
run: |
if [ ! -f "./basic-file.txt" ]; then
echo "Expected basic file does not exist"
exit 1
fi

# Verify .git folder
if [ ! -d "./.git" ]; then
echo "Expected ./.git folder to exist"
exit 1
fi

# Verify auth token
git config --global --add safe.directory "*"
git fetch --no-tags --depth=1 origin +refs/heads/main:refs/remotes/origin/main

# needed to make checkout post cleanup succeed
- name: Fix Checkout v3
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we running this again?

Copy link
Member Author

Choose a reason for hiding this comment

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

the second checkout will wipe the initial checkout and cause post-cleanup to fail...

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe i can do clean:false, let me try...

Copy link
Member Author

Choose a reason for hiding this comment

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

it doesn't work... 😢

Copy link
Contributor

Choose a reason for hiding this comment

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

lets add a quick comment if we can just to say # needed to make post run succeed

Choose a reason for hiding this comment

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

จัดการได้ตามความเหมาะสม เห็นชอบ ของชุมชน

uses: actions/checkout@v3
with:
path: v3
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ When Git 2.18 or higher is not in your PATH, falls back to the REST API to downl
#
# Default: false
submodules: ''

# Add repository path as safe.directory for Git global config by running `git
# config --global --add safe.directory <path>`
# Default: true
set-safe-directory: ''
```
<!-- end usage -->

Expand Down
3 changes: 2 additions & 1 deletion __test__/git-auth-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,8 @@ async function setup(testName: string): Promise<void> {
sshKey: sshPath ? 'some ssh private key' : '',
sshKnownHosts: '',
sshStrict: true,
workflowOrganizationId: 123456
workflowOrganizationId: 123456,
setSafeDirectory: true
}
}

Expand Down
1 change: 1 addition & 0 deletions __test__/input-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ describe('input-helper tests', () => {
expect(settings.repositoryName).toBe('some-repo')
expect(settings.repositoryOwner).toBe('some-owner')
expect(settings.repositoryPath).toBe(gitHubWorkspace)
expect(settings.setSafeDirectory).toBe(true)
})

it('qualifies ref', async () => {
Expand Down
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ inputs:
When the `ssh-key` input is not provided, SSH URLs beginning with `git@github.com:` are
converted to HTTPS.
default: false
set-safe-directory:
description: Add repository path as safe.directory for Git global config by running `git config --global --add safe.directory <path>`
default: true
runs:
using: node16
main: dist/index.js
Expand Down
51 changes: 39 additions & 12 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3592,7 +3592,7 @@ var __importStar = (this && this.__importStar) || function (mod) {
return result;
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.setSshKnownHostsPath = exports.setSshKeyPath = exports.setRepositoryPath = exports.SshKnownHostsPath = exports.SshKeyPath = exports.RepositoryPath = exports.IsPost = void 0;
exports.setSafeDirectory = exports.setSshKnownHostsPath = exports.setSshKeyPath = exports.setRepositoryPath = exports.SshKnownHostsPath = exports.SshKeyPath = exports.PostSetSafeDirectory = exports.RepositoryPath = exports.IsPost = void 0;
const coreCommand = __importStar(__webpack_require__(431));
/**
* Indicates whether the POST action is running
Expand All @@ -3602,6 +3602,10 @@ exports.IsPost = !!process.env['STATE_isPost'];
* The repository path for the POST action. The value is empty during the MAIN action.
*/
exports.RepositoryPath = process.env['STATE_repositoryPath'] || '';
/**
* The set-safe-directory for the POST action. The value is set if input: 'safe-directory' is set during the MAIN action.
*/
exports.PostSetSafeDirectory = process.env['STATE_setSafeDirectory'] === 'true';
/**
* The SSH key path for the POST action. The value is empty during the MAIN action.
*/
Expand Down Expand Up @@ -3631,6 +3635,13 @@ function setSshKnownHostsPath(sshKnownHostsPath) {
coreCommand.issueCommand('save-state', { name: 'sshKnownHostsPath' }, sshKnownHostsPath);
}
exports.setSshKnownHostsPath = setSshKnownHostsPath;
/**
* Save the sef-safe-directory input so the POST action can retrieve the value.
*/
function setSafeDirectory() {
coreCommand.issueCommand('save-state', { name: 'setSafeDirectory' }, 'true');
}
exports.setSafeDirectory = setSafeDirectory;
// Publish a variable so that when the POST action runs, it can determine it should run the cleanup logic.
// This is necessary since we don't have a separate entry point.
if (!exports.IsPost) {
Expand Down Expand Up @@ -6572,7 +6583,7 @@ class GitAuthHelper {
yield this.configureToken();
});
}
configureTempGlobalConfig(repositoryPath) {
configureTempGlobalConfig() {
var _a, _b;
return __awaiter(this, void 0, void 0, function* () {
// Already setup global config
Expand Down Expand Up @@ -6608,14 +6619,6 @@ class GitAuthHelper {
// Override HOME
core.info(`Temporarily overriding HOME='${this.temporaryHomePath}' before making global git config changes`);
this.git.setEnvironmentVariable('HOME', this.temporaryHomePath);
// Setup the workspace as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
core.info(`Adding working directory to the temporary git global config as a safe directory`);
yield this.git
.config('safe.directory', repositoryPath !== null && repositoryPath !== void 0 ? repositoryPath : this.settings.repositoryPath, true, true)
.catch(error => {
core.info(`Failed to initialize safe directory with error: ${error}`);
});
return newGitConfigPath;
});
}
Expand Down Expand Up @@ -7352,7 +7355,18 @@ function getSource(settings) {
try {
if (git) {
authHelper = gitAuthHelper.createAuthHelper(git, settings);
yield authHelper.configureTempGlobalConfig();
if (settings.setSafeDirectory) {
// Setup the repository path as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
yield authHelper.configureTempGlobalConfig();
core.info(`Adding repository directory to the temporary git global config as a safe directory`);
yield git
.config('safe.directory', settings.repositoryPath, true, true)
.catch(error => {
core.info(`Failed to initialize safe directory with error: ${error}`);
});
stateHelper.setSafeDirectory();
}
}
// Prepare existing directory, otherwise recreate
if (isExisting) {
Expand Down Expand Up @@ -7500,7 +7514,17 @@ function cleanup(repositoryPath) {
// Remove auth
const authHelper = gitAuthHelper.createAuthHelper(git);
try {
yield authHelper.configureTempGlobalConfig(repositoryPath);
if (stateHelper.PostSetSafeDirectory) {
// Setup the repository path as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
yield authHelper.configureTempGlobalConfig();
core.info(`Adding repository directory to the temporary git global config as a safe directory`);
yield git
.config('safe.directory', repositoryPath, true, true)
.catch(error => {
core.info(`Failed to initialize safe directory with error: ${error}`);
});
}
yield authHelper.removeAuth();
}
finally {
Expand Down Expand Up @@ -17303,6 +17327,9 @@ function getInputs() {
(core.getInput('persist-credentials') || 'false').toUpperCase() === 'TRUE';
// Workflow organization ID
result.workflowOrganizationId = yield workflowContextHelper.getOrganizationId();
// Set safe.directory in git global config.
result.setSafeDirectory =
(core.getInput('set-safe-directory') || 'true').toUpperCase() === 'TRUE';
return result;
});
}
Expand Down
19 changes: 2 additions & 17 deletions src/git-auth-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface IGitAuthHelper {
configureAuth(): Promise<void>
configureGlobalAuth(): Promise<void>
configureSubmoduleAuth(): Promise<void>
configureTempGlobalConfig(repositoryPath?: string): Promise<string>
configureTempGlobalConfig(): Promise<string>
removeAuth(): Promise<void>
removeGlobalConfig(): Promise<void>
}
Expand Down Expand Up @@ -81,7 +81,7 @@ class GitAuthHelper {
await this.configureToken()
}

async configureTempGlobalConfig(repositoryPath?: string): Promise<string> {
async configureTempGlobalConfig(): Promise<string> {
// Already setup global config
if (this.temporaryHomePath?.length > 0) {
return path.join(this.temporaryHomePath, '.gitconfig')
Expand Down Expand Up @@ -121,21 +121,6 @@ class GitAuthHelper {
)
this.git.setEnvironmentVariable('HOME', this.temporaryHomePath)

// Setup the workspace as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
core.info(
`Adding working directory to the temporary git global config as a safe directory`
)
await this.git
.config(
'safe.directory',
repositoryPath ?? this.settings.repositoryPath,
true,
true
)
.catch(error => {
core.info(`Failed to initialize safe directory with error: ${error}`)
})
return newGitConfigPath
}

Expand Down
35 changes: 33 additions & 2 deletions src/git-source-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,24 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
try {
if (git) {
authHelper = gitAuthHelper.createAuthHelper(git, settings)
await authHelper.configureTempGlobalConfig()
if (settings.setSafeDirectory) {
// Setup the repository path as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
await authHelper.configureTempGlobalConfig()
core.info(
`Adding repository directory to the temporary git global config as a safe directory`
)

await git
.config('safe.directory', settings.repositoryPath, true, true)
.catch(error => {
core.info(
`Failed to initialize safe directory with error: ${error}`
)
})

stateHelper.setSafeDirectory()
}
}

// Prepare existing directory, otherwise recreate
Expand Down Expand Up @@ -249,7 +266,21 @@ export async function cleanup(repositoryPath: string): Promise<void> {
// Remove auth
const authHelper = gitAuthHelper.createAuthHelper(git)
try {
await authHelper.configureTempGlobalConfig(repositoryPath)
if (stateHelper.PostSetSafeDirectory) {
// Setup the repository path as a safe directory, so if we pass this into a container job with a different user it doesn't fail
// Otherwise all git commands we run in a container fail
await authHelper.configureTempGlobalConfig()
core.info(
`Adding repository directory to the temporary git global config as a safe directory`
)

await git
.config('safe.directory', repositoryPath, true, true)
.catch(error => {
core.info(`Failed to initialize safe directory with error: ${error}`)
})
}

await authHelper.removeAuth()
} finally {
await authHelper.removeGlobalConfig()
Expand Down
5 changes: 5 additions & 0 deletions src/git-source-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,9 @@ export interface IGitSourceSettings {
* Organization ID for the currently running workflow (used for auth settings)
*/
workflowOrganizationId: number | undefined

/**
* Indicates whether to add repositoryPath as safe.directory in git global config
*/
setSafeDirectory: boolean
}
3 changes: 3 additions & 0 deletions src/input-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,8 @@ export async function getInputs(): Promise<IGitSourceSettings> {
// Workflow organization ID
result.workflowOrganizationId = await workflowContextHelper.getOrganizationId()

// Set safe.directory in git global config.
result.setSafeDirectory =
(core.getInput('set-safe-directory') || 'true').toUpperCase() === 'TRUE'
return result
}
13 changes: 13 additions & 0 deletions src/state-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ export const IsPost = !!process.env['STATE_isPost']
export const RepositoryPath =
(process.env['STATE_repositoryPath'] as string) || ''

/**
* The set-safe-directory for the POST action. The value is set if input: 'safe-directory' is set during the MAIN action.
*/
export const PostSetSafeDirectory =
(process.env['STATE_setSafeDirectory'] as string) === 'true'

/**
* The SSH key path for the POST action. The value is empty during the MAIN action.
*/
Expand Down Expand Up @@ -51,6 +57,13 @@ export function setSshKnownHostsPath(sshKnownHostsPath: string) {
)
}

/**
* Save the sef-safe-directory input so the POST action can retrieve the value.
*/
export function setSafeDirectory() {
coreCommand.issueCommand('save-state', {name: 'setSafeDirectory'}, 'true')
}

// Publish a variable so that when the POST action runs, it can determine it should run the cleanup logic.
// This is necessary since we don't have a separate entry point.
if (!IsPost) {
Expand Down