fix: URL-encode IAM policy documents in Query API responses (#1123)

AWS percent-encodes every policy document it returns. Spinifex returned
raw JSON, so a client decoding once read '+' as a space and '%41' as 'A',
giving perpetual Terraform drift, and a lone '%' failed the read.

Encode in the gateway wrappers for CreateRole, GetRole, ListRoles,
GetRolePolicy, GetUserPolicy, GetGroupPolicy, GetPolicyVersion and the
instance-profile reads. The service layer keeps raw JSON because STS and
spx admin parse it in-process. The web UI now decodes role inline
policies like every other document.

spinifex/gateway/iam/group.go

@@ -107,7 +107,12 @@ func GetGroupPolicy(accountID string, input *iam.GetGroupPolicyInput, svc handle
 	if input.PolicyName == nil || *input.PolicyName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetGroupPolicy(accountID, input)
+	out, err := svc.GetGroupPolicy(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	out.PolicyDocument = encodePolicyDocument(out.PolicyDocument)
+	return out, nil
 }
 
 func DeleteGroupPolicy(accountID string, input *iam.DeleteGroupPolicyInput, svc handlers_iam.IAMService) (*iam.DeleteGroupPolicyOutput, error) {

spinifex/gateway/iam/instance_profile.go

@@ -25,11 +25,21 @@ func GetInstanceProfile(accountID string, input *iam.GetInstanceProfileInput, sv
 	if input.InstanceProfileName == nil || *input.InstanceProfileName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetInstanceProfile(accountID, input)
+	out, err := svc.GetInstanceProfile(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeInstanceProfileDocuments(out.InstanceProfile)
+	return out, nil
 }
 
 func ListInstanceProfiles(accountID string, input *iam.ListInstanceProfilesInput, svc handlers_iam.IAMService) (*iam.ListInstanceProfilesOutput, error) {
-	return svc.ListInstanceProfiles(accountID, input)
+	out, err := svc.ListInstanceProfiles(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeInstanceProfileDocuments(out.InstanceProfiles...)
+	return out, nil
 }
 
 // DeleteInstanceProfile refuses to delete a profile still referenced by a live
@@ -72,7 +82,12 @@ func ListInstanceProfilesForRole(accountID string, input *iam.ListInstanceProfil
 	if input.RoleName == nil || *input.RoleName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.ListInstanceProfilesForRole(accountID, input)
+	out, err := svc.ListInstanceProfilesForRole(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeInstanceProfileDocuments(out.InstanceProfiles...)
+	return out, nil
 }
 
 func AddRoleToInstanceProfile(accountID string, input *iam.AddRoleToInstanceProfileInput, svc handlers_iam.IAMService) (*iam.AddRoleToInstanceProfileOutput, error) {

spinifex/gateway/iam/policy.go

@@ -32,7 +32,14 @@ func GetPolicyVersion(accountID string, input *iam.GetPolicyVersionInput, svc ha
 	if input.VersionId == nil || *input.VersionId == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetPolicyVersion(accountID, input)
+	out, err := svc.GetPolicyVersion(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	if out.PolicyVersion != nil {
+		out.PolicyVersion.Document = encodePolicyDocument(out.PolicyVersion.Document)
+	}
+	return out, nil
 }
 
 func ListPolicyVersions(accountID string, input *iam.ListPolicyVersionsInput, svc handlers_iam.IAMService) (*iam.ListPolicyVersionsOutput, error) {

spinifex/gateway/iam/policy_document.go

@@ -0,0 +1,35 @@
+package gateway_iam
+
+import (
+	"net/url"
+	"strings"
+
+	"github.com/aws/aws-sdk-go/service/iam"
+)
+
+// encodePolicyDocument percent-encodes a policy document the way the IAM Query
+// API returns it: everything but letters, digits and -_.~ escaped, space as %20.
+// The service layer keeps raw JSON because STS and spx admin parse it in-process.
+func encodePolicyDocument(doc *string) *string {
+	if doc == nil {
+		return nil
+	}
+	encoded := strings.ReplaceAll(url.QueryEscape(*doc), "+", "%20")
+	return &encoded
+}
+
+func encodeRoleDocuments(roles ...*iam.Role) {
+	for _, r := range roles {
+		if r != nil {
+			r.AssumeRolePolicyDocument = encodePolicyDocument(r.AssumeRolePolicyDocument)
+		}
+	}
+}
+
+func encodeInstanceProfileDocuments(profiles ...*iam.InstanceProfile) {
+	for _, p := range profiles {
+		if p != nil {
+			encodeRoleDocuments(p.Roles...)
+		}
+	}
+}

spinifex/gateway/iam/policy_document_test.go

@@ -0,0 +1,176 @@
+package gateway_iam
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/iam"
+	handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// rawDoc holds the characters a decode-once client would corrupt if the
+// document went out unencoded: '+' reads as space, "%41" as "A", a lone '%' fails.
+const (
+	rawDoc     = `{"Sid":"a+b%41c d/x:y %"}`
+	encodedDoc = `%7B%22Sid%22%3A%22a%2Bb%2541c%20d%2Fx%3Ay%20%25%22%7D`
+)
+
+// rawDocIAMService answers every document-bearing read with rawDoc, as the
+// service layer stores and returns it.
+type rawDocIAMService struct {
+	handlers_iam.IAMService
+}
+
+func rawDocRole() *iam.Role {
+	return &iam.Role{RoleName: aws.String("r"), AssumeRolePolicyDocument: aws.String(rawDoc)}
+}
+
+func rawDocProfile() *iam.InstanceProfile {
+	return &iam.InstanceProfile{Roles: []*iam.Role{rawDocRole()}}
+}
+
+func (rawDocIAMService) CreateRole(string, *iam.CreateRoleInput) (*iam.CreateRoleOutput, error) {
+	return &iam.CreateRoleOutput{Role: rawDocRole()}, nil
+}
+func (rawDocIAMService) GetRole(string, *iam.GetRoleInput) (*iam.GetRoleOutput, error) {
+	return &iam.GetRoleOutput{Role: rawDocRole()}, nil
+}
+func (rawDocIAMService) ListRoles(string, *iam.ListRolesInput) (*iam.ListRolesOutput, error) {
+	return &iam.ListRolesOutput{Roles: []*iam.Role{rawDocRole(), rawDocRole()}}, nil
+}
+func (rawDocIAMService) GetRolePolicy(string, *iam.GetRolePolicyInput) (*iam.GetRolePolicyOutput, error) {
+	return &iam.GetRolePolicyOutput{PolicyDocument: aws.String(rawDoc)}, nil
+}
+func (rawDocIAMService) GetUserPolicy(string, *iam.GetUserPolicyInput) (*iam.GetUserPolicyOutput, error) {
+	return &iam.GetUserPolicyOutput{PolicyDocument: aws.String(rawDoc)}, nil
+}
+func (rawDocIAMService) GetGroupPolicy(string, *iam.GetGroupPolicyInput) (*iam.GetGroupPolicyOutput, error) {
+	return &iam.GetGroupPolicyOutput{PolicyDocument: aws.String(rawDoc)}, nil
+}
+func (rawDocIAMService) GetPolicyVersion(string, *iam.GetPolicyVersionInput) (*iam.GetPolicyVersionOutput, error) {
+	return &iam.GetPolicyVersionOutput{PolicyVersion: &iam.PolicyVersion{Document: aws.String(rawDoc)}}, nil
+}
+func (rawDocIAMService) GetInstanceProfile(string, *iam.GetInstanceProfileInput) (*iam.GetInstanceProfileOutput, error) {
+	return &iam.GetInstanceProfileOutput{InstanceProfile: rawDocProfile()}, nil
+}
+func (rawDocIAMService) ListInstanceProfiles(string, *iam.ListInstanceProfilesInput) (*iam.ListInstanceProfilesOutput, error) {
+	return &iam.ListInstanceProfilesOutput{InstanceProfiles: []*iam.InstanceProfile{rawDocProfile(), rawDocProfile()}}, nil
+}
+func (rawDocIAMService) ListInstanceProfilesForRole(string, *iam.ListInstanceProfilesForRoleInput) (*iam.ListInstanceProfilesForRoleOutput, error) {
+	return &iam.ListInstanceProfilesForRoleOutput{InstanceProfiles: []*iam.InstanceProfile{rawDocProfile()}}, nil
+}
+
+func roleDocs(roles ...*iam.Role) []*string {
+	var docs []*string
+	for _, r := range roles {
+		docs = append(docs, r.AssumeRolePolicyDocument)
+	}
+	return docs
+}
+
+func profileDocs(profiles ...*iam.InstanceProfile) []*string {
+	var docs []*string
+	for _, p := range profiles {
+		docs = append(docs, roleDocs(p.Roles...)...)
+	}
+	return docs
+}
+
+// TestPolicyDocumentsAreURLEncoded pins the percent-encoding AWS applies to
+// every document it returns over the Query API.
+func TestPolicyDocumentsAreURLEncoded(t *testing.T) {
+	svc := rawDocIAMService{}
+	name := aws.String("n")
+	tests := []struct {
+		action string
+		call   func() ([]*string, error)
+	}{
+		{"CreateRole", func() ([]*string, error) {
+			out, err := CreateRole(testAccountID, &iam.CreateRoleInput{RoleName: name, AssumeRolePolicyDocument: aws.String(rawDoc)}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return roleDocs(out.Role), nil
+		}},
+		{"GetRole", func() ([]*string, error) {
+			out, err := GetRole(testAccountID, &iam.GetRoleInput{RoleName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return roleDocs(out.Role), nil
+		}},
+		{"ListRoles", func() ([]*string, error) {
+			out, err := ListRoles(testAccountID, &iam.ListRolesInput{}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return roleDocs(out.Roles...), nil
+		}},
+		{"GetRolePolicy", func() ([]*string, error) {
+			out, err := GetRolePolicy(testAccountID, &iam.GetRolePolicyInput{RoleName: name, PolicyName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return []*string{out.PolicyDocument}, nil
+		}},
+		{"GetUserPolicy", func() ([]*string, error) {
+			out, err := GetUserPolicy(testAccountID, &iam.GetUserPolicyInput{UserName: name, PolicyName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return []*string{out.PolicyDocument}, nil
+		}},
+		{"GetGroupPolicy", func() ([]*string, error) {
+			out, err := GetGroupPolicy(testAccountID, &iam.GetGroupPolicyInput{GroupName: name, PolicyName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return []*string{out.PolicyDocument}, nil
+		}},
+		{"GetPolicyVersion", func() ([]*string, error) {
+			out, err := GetPolicyVersion(testAccountID, &iam.GetPolicyVersionInput{PolicyArn: name, VersionId: aws.String("v1")}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return []*string{out.PolicyVersion.Document}, nil
+		}},
+		{"GetInstanceProfile", func() ([]*string, error) {
+			out, err := GetInstanceProfile(testAccountID, &iam.GetInstanceProfileInput{InstanceProfileName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return profileDocs(out.InstanceProfile), nil
+		}},
+		{"ListInstanceProfiles", func() ([]*string, error) {
+			out, err := ListInstanceProfiles(testAccountID, &iam.ListInstanceProfilesInput{}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return profileDocs(out.InstanceProfiles...), nil
+		}},
+		{"ListInstanceProfilesForRole", func() ([]*string, error) {
+			out, err := ListInstanceProfilesForRole(testAccountID, &iam.ListInstanceProfilesForRoleInput{RoleName: name}, svc)
+			if err != nil {
+				return nil, err
+			}
+			return profileDocs(out.InstanceProfiles...), nil
+		}},
+	}
+	for _, tc := range tests {
+		t.Run(tc.action, func(t *testing.T) {
+			docs, err := tc.call()
+			require.NoError(t, err)
+			require.NotEmpty(t, docs)
+			for _, doc := range docs {
+				require.NotNil(t, doc)
+				assert.Equal(t, encodedDoc, *doc)
+				decoded, err := url.QueryUnescape(*doc)
+				require.NoError(t, err)
+				assert.Equal(t, rawDoc, decoded, "a client decoding once must recover the stored document")
+			}
+		})
+	}
+}

spinifex/gateway/iam/role.go

@@ -15,18 +15,33 @@ func CreateRole(accountID string, input *iam.CreateRoleInput, svc handlers_iam.I
 	if input.AssumeRolePolicyDocument == nil || *input.AssumeRolePolicyDocument == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.CreateRole(accountID, input)
+	out, err := svc.CreateRole(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeRoleDocuments(out.Role)
+	return out, nil
 }
 
 func GetRole(accountID string, input *iam.GetRoleInput, svc handlers_iam.IAMService) (*iam.GetRoleOutput, error) {
 	if input.RoleName == nil || *input.RoleName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetRole(accountID, input)
+	out, err := svc.GetRole(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeRoleDocuments(out.Role)
+	return out, nil
 }
 
 func ListRoles(accountID string, input *iam.ListRolesInput, svc handlers_iam.IAMService) (*iam.ListRolesOutput, error) {
-	return svc.ListRoles(accountID, input)
+	out, err := svc.ListRoles(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	encodeRoleDocuments(out.Roles...)
+	return out, nil
 }
 
 func DeleteRole(accountID string, input *iam.DeleteRoleInput, svc handlers_iam.IAMService) (*iam.DeleteRoleOutput, error) {
@@ -100,7 +115,12 @@ func GetRolePolicy(accountID string, input *iam.GetRolePolicyInput, svc handlers
 	if input.PolicyName == nil || *input.PolicyName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetRolePolicy(accountID, input)
+	out, err := svc.GetRolePolicy(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	out.PolicyDocument = encodePolicyDocument(out.PolicyDocument)
+	return out, nil
 }
 
 func DeleteRolePolicy(accountID string, input *iam.DeleteRolePolicyInput, svc handlers_iam.IAMService) (*iam.DeleteRolePolicyOutput, error) {

spinifex/gateway/iam/user.go

@@ -53,7 +53,12 @@ func GetUserPolicy(accountID string, input *iam.GetUserPolicyInput, svc handlers
 	if input.PolicyName == nil || *input.PolicyName == "" {
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
-	return svc.GetUserPolicy(accountID, input)
+	out, err := svc.GetUserPolicy(accountID, input)
+	if err != nil {
+		return nil, err
+	}
+	out.PolicyDocument = encodePolicyDocument(out.PolicyDocument)
+	return out, nil
 }
 
 func DeleteUserPolicy(accountID string, input *iam.DeleteUserPolicyInput, svc handlers_iam.IAMService) (*iam.DeleteUserPolicyOutput, error) {

spinifex/handlers/iam/roles.go

@@ -396,9 +396,7 @@ func (s *IAMServiceImpl) PutRolePolicy(accountID string, input *iam.PutRolePolic
 	return &iam.PutRolePolicyOutput{}, nil
 }
 
-// GetRolePolicy returns a role's inline policy document by name.
-// Returns the document as a raw JSON string, matching how GetRole returns
-// AssumeRolePolicyDocument; AWS URL-encodes it, we follow the in-repo convention.
+// GetRolePolicy returns a role's inline policy document by name, as raw JSON.
 func (s *IAMServiceImpl) GetRolePolicy(accountID string, input *iam.GetRolePolicyInput) (*iam.GetRolePolicyOutput, error) {
 	ctx := context.Background()
 	roleName := *input.RoleName

spinifex/services/spinifexui/frontend/src/lib/json.test.ts

@@ -42,24 +42,20 @@ describe("formatJson", () => {
 describe("decodePolicyDocument", () => {
   it("decodes and pretty-prints a URL-encoded document", () => {
     const encoded = encodeURIComponent('{"Version":"2012-10-17"}')
-    expect(decodePolicyDocument(encoded, true)).toBe(
+    expect(decodePolicyDocument(encoded)).toBe(
       '{\n  "Version": "2012-10-17"\n}',
     )
   })
 
-  it("pretty-prints a raw JSON document without decoding", () => {
-    expect(decodePolicyDocument('{"a":1}', false)).toBe('{\n  "a": 1\n}')
-  })
-
-  it("leaves literal percent sequences intact for raw documents", () => {
-    const raw = '{"Resource":"arn:aws:s3:::bucket/logs%2Fpath"}'
-    expect(decodePolicyDocument(raw, false)).toBe(
-      '{\n  "Resource": "arn:aws:s3:::bucket/logs%2Fpath"\n}',
+  it("decodes exactly once, keeping a literal + and %41", () => {
+    const encoded = encodeURIComponent('{"Resource":"a+b%41c d"}')
+    expect(decodePolicyDocument(encoded)).toBe(
+      '{\n  "Resource": "a+b%41c d"\n}',
     )
   })
 
   it("returns the original text when it is not JSON", () => {
-    expect(decodePolicyDocument("not json", true)).toBe("not json")
+    expect(decodePolicyDocument("not json")).toBe("not json")
   })
 })
 

spinifex/services/spinifexui/frontend/src/lib/json.ts

@@ -19,21 +19,14 @@ export function formatJson(value: string): string | null {
   }
 }
 
-// Normalise an AWS policy document for display in an editor. Most principals
-// return the document URL-encoded, but roles return raw JSON, so callers pass
-// `encoded` to control decoding — decoding raw JSON could corrupt literal
-// percent sequences. The result is pretty-printed when it parses.
-export function decodePolicyDocument(
-  document: string,
-  encoded: boolean,
-): string {
-  let text = document
-  if (encoded) {
-    try {
-      text = decodeURIComponent(document)
-    } catch {
-      text = document
-    }
+// Normalise an AWS policy document for display in an editor. IAM returns
+// documents URL-encoded; the result is pretty-printed when it parses.
+export function decodePolicyDocument(document: string): string {
+  let text: string
+  try {
+    text = decodeURIComponent(document)
+  } catch {
+    text = document
   }
   return formatJson(text) ?? text
 }

spinifex/services/spinifexui/frontend/src/queries/iam.test.ts

@@ -365,9 +365,9 @@ describe("queryFn", () => {
     })
   })
 
-  it("iamRolePolicyQueryOptions sends GetRolePolicyCommand and formats raw JSON", async () => {
+  it("iamRolePolicyQueryOptions sends GetRolePolicyCommand and decodes document", async () => {
     mockSend.mockResolvedValueOnce({
-      PolicyDocument: '{"Version":"2012-10-17"}',
+      PolicyDocument: encodeURIComponent('{"Version":"2012-10-17"}'),
     })
     const document = await callQueryFn(
       iamRolePolicyQueryOptions("my-role", "s3-read"),

spinifex/services/spinifexui/frontend/src/queries/iam.ts

@@ -127,7 +127,7 @@ export const iamUserPolicyQueryOptions = (
       })
       const result = await getIamClient().send(command)
       return result.PolicyDocument
-        ? decodePolicyDocument(result.PolicyDocument, true)
+        ? decodePolicyDocument(result.PolicyDocument)
         : ""
     },
     staleTime: 300_000,
@@ -187,7 +187,7 @@ export const iamRolePolicyQueryOptions = (
       })
       const result = await getIamClient().send(command)
       return result.PolicyDocument
-        ? decodePolicyDocument(result.PolicyDocument, false)
+        ? decodePolicyDocument(result.PolicyDocument)
         : ""
     },
     staleTime: 300_000,
@@ -280,7 +280,7 @@ export const iamGroupPolicyQueryOptions = (
       })
       const result = await getIamClient().send(command)
       return result.PolicyDocument
-        ? decodePolicyDocument(result.PolicyDocument, true)
+        ? decodePolicyDocument(result.PolicyDocument)
         : ""
     },
     staleTime: 300_000,

tests/integration/iam_groups_test.go

@@ -3,6 +3,7 @@
 package integration
 
 import (
+	"net/url"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -341,7 +342,7 @@ func TestIAMGroupsLifecycle(t *testing.T) {
 	})
 	require.NoError(t, err, "idempotent re-put")
 
-	// GetGroupPolicy — round-trips the stored document (raw, not URL-encoded).
+	// GetGroupPolicy — returns the stored document URL-encoded, as AWS does.
 	inlineGot, err := iamCli.GetGroupPolicy(&iam.GetGroupPolicyInput{
 		GroupName:  aws.String(grpLifecycleGroup),
 		PolicyName: aws.String(grpLifecycleInline),
@@ -349,7 +350,9 @@ func TestIAMGroupsLifecycle(t *testing.T) {
 	require.NoError(t, err, "get-group-policy")
 	require.Equal(t, grpLifecycleGroup, aws.StringValue(inlineGot.GroupName))
 	require.Equal(t, grpLifecycleInline, aws.StringValue(inlineGot.PolicyName))
-	require.JSONEq(t, groupDescribeRegionsPolicy, aws.StringValue(inlineGot.PolicyDocument),
+	inlineDoc, err := url.QueryUnescape(aws.StringValue(inlineGot.PolicyDocument))
+	require.NoError(t, err, "get-group-policy document must be URL-encoded")
+	require.JSONEq(t, groupDescribeRegionsPolicy, inlineDoc,
 		"get-group-policy must round-trip the stored document")
 
 	_, err = iamCli.GetGroupPolicy(&iam.GetGroupPolicyInput{