Skip to content

Fix nil pointer validation issues in Power Platform Terraform provider #830

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

Merged
merged 6 commits into from
Jun 18, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 5, 2025

This PR addresses three critical nil pointer validation issues that could lead to runtime panics in the Power Platform Terraform provider.

Issues Fixed

Issue 1: Missing nil check for operation location header

File: internal/services/application/api_application.go

The InstallApplicationInEnvironment method was not validating that the operation location header was present before using it, which could cause downstream HTTP requests with empty URLs or crashes.

Before:

operationLocationHeader := response.GetHeader(constants.HEADER_OPERATION_LOCATION)
tflog.Debug(ctx, "Operation Location Header: "+operationLocationHeader)

_, err = url.Parse(operationLocationHeader) // Could panic if header is empty

After:

operationLocationHeader := response.GetHeader(constants.HEADER_OPERATION_LOCATION)
if operationLocationHeader == "" {
    tflog.Error(ctx, "Missing operation location header in response")
    return "", errors.New("missing operation location header in response")
}
tflog.Debug(ctx, "Operation Location Header: "+operationLocationHeader)

_, err = url.Parse(operationLocationHeader)
if err != nil {
    tflog.Error(ctx, "Error parsing location header: "+err.Error())
    return "", err
}

Issue 2: Repetitive null/unknown validation patterns

Files: internal/services/environment/models.go, internal/services/environment_settings/models.go

Created a reusable helpers.IsKnown() function to replace verbose null/unknown checks throughout the codebase.

Before:

if !environmentSource.EnvironmentGroupId.IsNull() && !environmentSource.EnvironmentGroupId.IsUnknown() {
    environmentDto.Properties.ParentEnvironmentGroup = &ParentEnvironmentGroupDto{Id: environmentSource.EnvironmentGroupId.ValueString()}
}

After:

if helpers.IsKnown(environmentSource.EnvironmentGroupId) {
    environmentDto.Properties.ParentEnvironmentGroup = &ParentEnvironmentGroupDto{Id: environmentSource.EnvironmentGroupId.ValueString()}
}

Issue 3: Missing nil/zero-value validation for critical fields

File: internal/services/authorization/resource_user.go

Added comprehensive validation for plan and state objects in all CRUD operations to prevent nil pointer dereferences.

Before:

var plan *UserResourceModel
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() {
    return
}

// Could panic if plan is nil or fields are null/unknown
hasEnvDataverse, err := r.UserClient.EnvironmentHasDataverse(ctx, plan.EnvironmentId.ValueString())

After:

var plan *UserResourceModel
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() || plan == nil {
    return
}

// Validate required fields before dereferencing
if plan.EnvironmentId.IsUnknown() || plan.EnvironmentId.IsNull() {
    resp.Diagnostics.AddError("Invalid Configuration", "Environment ID is required and cannot be null or unknown")
    return
}
if plan.AadId.IsUnknown() || plan.AadId.IsNull() {
    resp.Diagnostics.AddError("Invalid Configuration", "AAD ID is required and cannot be null or unknown")
    return
}

hasEnvDataverse, err := r.UserClient.EnvironmentHasDataverse(ctx, plan.EnvironmentId.ValueString())

Testing

  • All existing unit tests continue to pass
  • Linter passes with no issues
  • Documentation generated successfully
  • Tested all modified services: application, authorization, environment, and environment_settings

Impact

These changes improve the robustness and reliability of the provider by:

  • Preventing runtime panics from nil pointer dereferences
  • Providing clear error messages for invalid configurations
  • Reducing code duplication through the new helpers.IsKnown() utility function
  • Maintaining backward compatibility with existing functionality

Fixes #815.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits June 5, 2025 07:07
…els, authorization resource, and environment settings

Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] [Copilot] Nil Pointer Validation Issues Fix nil pointer validation issues in Power Platform Terraform provider Jun 5, 2025
@Copilot Copilot AI requested a review from mawasile June 5, 2025 07:11
Copilot finished work on behalf of mawasile June 5, 2025 07:11
@mawasile mawasile marked this pull request as ready for review June 5, 2025 09:17
@Copilot Copilot AI review requested due to automatic review settings June 5, 2025 09:17
@mawasile mawasile requested a review from a team as a code owner June 5, 2025 09:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR strengthens nil pointer validation across the Terraform provider to prevent runtime panics by introducing a reusable helper, adding missing checks, and improving error handling.

  • Replaced repetitive null/unknown checks with a new helpers.IsKnown function
  • Added missing header existence and parse error handling in the application client
  • Enforced required field validations in user resource CRUD methods

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/services/application/api_application.go Added missing check for operation location header and error handling
internal/helpers/config.go Introduced helpers.IsKnown to encapsulate null/unknown checks
internal/services/environment_settings/models.go Replaced verbose null/unknown patterns with helpers.IsKnown
internal/services/environment/models.go Simplified environment model validations using helpers.IsKnown
internal/services/authorization/resource_user.go Added nil/unknown checks for planning and state fields in CRUD operations
.changes/unreleased/fixed-20250605-070828.yaml Documented the fix for Issue #815
Comments suppressed due to low confidence (2)

internal/services/application/api_application.go:281

  • Correct the typo in the debug message: change "Opeartion Location Header" to "Operation Location Header".
tflog.Debug(ctx, "Opeartion Location Header: "+operationLocationHeader)

internal/helpers/config.go:89

  • Add unit tests for the new IsKnown helper to verify its behavior with null, unknown, and defined attribute values.
func IsKnown(value attr.Value) bool {

@mawasile mawasile added ai assisted Pull requests that have be fully or partially implemented using Copilot or AI ai found Issues and Bugs that were found using AI copilot fixed using GitHub copilot autonomous agent labels Jun 9, 2025
mawasile and others added 2 commits June 9, 2025 14:05
…orization resource

Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
Copilot finished work on behalf of mawasile June 9, 2025 12:17
@mawasile mawasile merged commit cf12348 into main Jun 18, 2025
15 checks passed
@mawasile mawasile deleted the copilot/fix-815 branch June 18, 2025 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai assisted Pull requests that have be fully or partially implemented using Copilot or AI ai found Issues and Bugs that were found using AI copilot fixed using GitHub copilot autonomous agent
Projects
None yet
Development

Error loading sessions

Retrying...

Successfully merging this pull request may close these issues.

[Copilot] Nil Pointer Validation Issues
4 participants