fix: enforce relation databag read permissions on all access paths

[tonyandrewmeyer]

A charm that touches its own application databag from a non-leader unit gets this, with nothing in it pointing at leadership:

ops.model.ModelError: ERROR permission denied (unauthorized access)

ops already has a much better error for exactly this case (ops/model.py:2049):

<unit> is not leader and cannot read its own application databag

However, the charm never sees it. RelationDataContent.__getitem__ calls _validate_read(), but _GenericLazyMapping.__contains__, __iter__ and __len__ go straight to _data → _load() → the hook tool with no validation. update()'s change detection uses key not in self, so it took the unvalidated path and surfaced Juju's raw error.

This applies both with Relation.save() and also direct usage of update().

This PR makes two changes:

We now validate on every read path. _load() now calls _validate_read(), so no read can bypass the check. __getitem__ and __repr__ validate only when the data is already cached (_validate_cached_read), otherwise _load does it, and doing both would mean a redundant leadership check. That preserves today's behaviour for a unit that loses leadership after a read, at no extra cost. _validate_read() is almost always very quick (it's checking a few attributes, except in the case where a fresh leadership check is required).

We now check write permission before reading in update(). _validate_write ran after the diff loop, so a follower writing application data failed on an incidental read that change detection happens to need, rather than on the write it actually asked for. It now raises <unit> is not leader and cannot write application data.

[tonyandrewmeyer]

This is because there's a behaviour change. You're allowed to access relation data outside of a hook in Harness without setting up things so that you should have access to it. This could break real tests. At minimum it would need a hyrum run, and I wonder that it's too risky to do at all in a minor / point release.

[tonyandrewmeyer]

This is a behaviour change in that if there was something that was removed by the change but has type problems (something we can't store, for example) it will break now and wouldn't have before. I think in this case that's probably ok, it seems a weird situation and is arguably buggy behaviour anyway. But it would be worth checking that nothing we know of does break because of this.

[tonyandrewmeyer]

The change here is exactly what this comment is about. Basically, the comment was acknowledging the buggy (or limited) behaviour and the test pinned that behaviour. The PR updates it so that we're doing the proper thing instead.